mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Luben Tuikov <luben_tuikov@adaptec.com>
To: James Bottomley <James.Bottomley@SteelEye.com>, luben_tuikov@adaptec.com
Cc: Andrew Morton <akpm@osdl.org>, Jim Houston <jim.houston@ccur.com>,
	Linux Kernel <linux-kernel@vger.kernel.org>,
	SCSI Mailing List <linux-scsi@vger.kernel.org>,
	Dave Jones <davej@redhat.com>, Jeff Garzik <jgarzik@pobox.com>
Subject: Re: [PATCH 2.6.12.5 1/2] lib: allow idr to be used in irq context
Date: Sun, 21 Aug 2005 17:33:25 -0700 (PDT)	[thread overview]
Message-ID: <20050822003325.33507.qmail@web51613.mail.yahoo.com> (raw)
In-Reply-To: <1124661830.5068.19.camel@mulgrave>

--- James Bottomley <James.Bottomley@SteelEye.com> wrote:
> On Sun, 2005-08-21 at 10:27 -0700, Luben Tuikov wrote:
> > Is this the only use _you_ could find for a *radix tree*? ;-)
> > Since of course sd.c uses it just as an enumeration, according to
> > you this must be the only use? :-)
> 
> And Dicto Simpliciter to you too.
> 
> > It was designed as a general purpose id to pointer translation
> > service, just as the comment in it says.
> 
> idr was specifically designed with unlocked pre-allocation in mind.  The
> change you're proposing wants to allow pre allocation from irq context.
> This really doesn't look like a good change because the whole point of
> pre allocation is to do it at a point in time when the system can sleep
> if it has to.

No preallocation is done from IRQ context.  Do not spread FUD.
It seems to me that you're unaware how IDR works and unaware how
the driver works.

See the original patch submission -- I do explain why it is needed,
and show exactly how it breaks.  You may also want to look at the idr.c
code.  There is no reason not to promote the lock to an irq spinlock,
again, since it protects only data manipulation, as it should.

In fact, when you start writing code and actually face problems,
and start looking for the right tools, then you'll see it.  Right now
this is a useless argument and you should drop it.

> What I don't understand is why you need to pre allocate this tag from
> irq context.  Best practise is to allocate when you have user context
> but make use of it in IRQ context.

No preallocation is done in IRQ context.  See above.

Furthermore, you should know that a slab cache *settles* very
quickly, especially for IO.  You should've learned this when
I posted my results of SCSI Core using the slab cache for commands
5 years ago, before that code was in SCSI Core.

James, is this the most important thing for SCSI Core for the maintaner of
SCSI Core to discuss?

IDR's use?  A general library utility's use in a driver?
Is this what the SCSI Core maintainer will use to reject a driver's
acceptance?

When so many things have been said in the last week on this mailing
list, being in direct importance to SCSI Core: error handling
being one of them, another is HCIL usage, and other vendor
provided labels for LUs, native target support, etc.

> > > However, there is an infrastructure in the block layer called the
> > > generic tag infrastructure which was designed precisely for this purpose
> > > and which is designed to operate in IRQ context.
> > 
> > James, I'm sure you're well aware that,
> >    - a request_queue is LU-bound,
> >    - a SCSI _transport_ (*ANY*) can _only_ address domain devices, but
> >      _not_ LUs.  LUs are *not* seen on the domain.
> > 
> > See the different associations?  Then why are you posting such emails?
> 
> So you're planning to do some type of tag command queueing outside of
> the block framework?  Could you just look at what it would take to do it
> within the existing framework first?  I seem to have wasted quite a bit
> of time recently pulling spurious queueing code out of Adaptec drivers.

No one is implementing tag command queueing.  Why are you spreading FUD
into making people think that if you use tags, you must be doing
tagged command _queueing_?

Here:
    Tags do _not_ imply TCQ.
    TCQ does imply Tags.

> Remember that the code is adaptable ... we have non-block devices that
> use block queues for instance.

Completely irrelevant for a transport driver.  Stop throwing red-herring.

Instead let's start discussing matters SCSI, which have been due for
the last 5 years, which I had hoped you'd _lead_ or at least
*listen and take paches* into SCSI Core, you know, like other
maintainers do.  Those are:
    - 8 byte LUNs, opaque to SCSI Core,
    - native target (domain device) support,
    - native LU discovery,
    - HCIL becoming just one of _many_ possible labels for an LU, whereby
      vendors provide them.

*This* is what is important and relevant to SCSI Core.  Not to mention,
error handling, etc.

Now let's drop this and start discussing things which have been
a pain for the last 5 years, since the first SAM LLDD (iSCSI)
was introduced to SCSI Core (at least not publicly).

Let's take SCSI Core into the 21st century, shall we?  Please?

   Luben


  reply	other threads:[~2005-08-22 23:13 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-08-16 22:03 Luben Tuikov
2005-08-17 16:01 ` Jim Houston
2005-08-21  8:25   ` Andrew Morton
2005-08-21 15:49     ` Luben Tuikov
2005-08-21 16:06       ` James Bottomley
2005-08-21 17:27         ` Luben Tuikov
2005-08-21 22:03           ` James Bottomley
2005-08-22  0:33             ` Luben Tuikov [this message]
2005-08-22  3:15               ` James Bottomley
2005-08-22  3:52                 ` Andrew Morton
2005-08-22 14:28                   ` James Bottomley
2005-08-22 16:51                     ` Luben Tuikov
2005-08-22 21:53                     ` James Bottomley
2005-08-22 22:09                       ` Andrew Morton
2005-08-23 17:15                         ` James Bottomley
2005-08-22 16:33                   ` Luben Tuikov
2005-08-22 14:06         ` Luben Tuikov
2005-08-21 20:40 Luben Tuikov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20050822003325.33507.qmail@web51613.mail.yahoo.com \
    --to=luben_tuikov@adaptec.com \
    --cc=James.Bottomley@SteelEye.com \
    --cc=akpm@osdl.org \
    --cc=davej@redhat.com \
    --cc=jgarzik@pobox.com \
    --cc=jim.houston@ccur.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®