All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hannes Reinecke <hare@suse.de>
To: James.Smart@Emulex.Com
Cc: linux-scsi@vger.kernel.org
Subject: Re: [PATCH] scsi : set target can_queue from devinfo flags
Date: Wed, 14 May 2008 17:01:57 +0200	[thread overview]
Message-ID: <482AFEE5.2090300@suse.de> (raw)
In-Reply-To: <482AF9B0.7090307@emulex.com>

Hi James,

James Smart wrote:
> 
> 
> Hannes Reinecke wrote:
>>> This patch was cut against scsi-misc-2.6, and depends on Mike 
>>> Christies patches contained in the original thread.
>>>
>> Hmph.
>>
>> I don't quite agree with this one.
>> For once, /proc/scsi/scsi has been marked as 'obsolete' for quite some 
>> time now,
>> so adding other usages to this is of questionable value.
> 
> Um... I didn't think I added a new use for it.  The existing data had to
> be extended by an additional argument.
> 
Well, extending the existing structure comes quite close to adding another
usage ...

>>
>> And we've actually have a similar issue when developing the SCSI 
>> device_handler stuff where we also have a device list to maintain.
>>
>> Seeing there is quite some overlap between those two cases I think we 
>> should come up with a way of handling these things properly, ie tied
>> into sysfs.
> 
> Ok - but I see it as a two part process. I am not signing up for replacing
> the device list infrastructure.  But, now that there is target queue depth
> management in the midlayer, I believe we should be taking advantage of it.
> I'd rather see the additional field go in, then see a separate effort to
> replace/collapse the device lists...
> 
Ok, agreed. It makes sense to have it now and update the infrastructure
later.

>> So, what we should do here is
>> a) add a 'can_queue' sysfs attribute to the starget (which we can 
>> nowadays, as the starget is a proper sysfs object)
> 
> Um, it exists under the /sys/devices tree (the base object) but there is no
> class representation. Are you requesting this goes on the base object ?
> I thought we avoided this.
Yes. Although I can't figure out _why_.
We nowadays have perfectly valid scsi_target kobj, which can (and will if
CONFIG_SYSFS_DEPRECATED is not set) be displayed in sysfs.

> I guess it can, but as it's scsi-ish, I would
> think it more appropriate to formally create a scsi_target class and put
> scsi attributes there.  (but I was avoiding that too)
> 
We don't have to. It's just the once upon a time a class object was
used to access the underlying object. But nowadays the destinction
between class_objects and 'normal' objects is gone so there's hardly
any point in creating one.

I personally don't have a problem with putting the 'generic' SCSI
sysfs attributes into the kobj itself and getting rid of the class
devices for it (or rather, make it a link).
And, actually, plan to do some patches for it :-)

But that's slightly off-topic here.
And as we don't have a consensus about the future sysfs layout
yet it hardly makes any sense to discuss the placement of target
specific attributes.

So:
Acked-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2008-05-14 15:01 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-13 17:45 [PATCH] scsi : set target can_queue from devinfo flags James Smart
2008-05-14  6:34 ` Hannes Reinecke
2008-05-14 14:39   ` James Smart
2008-05-14 15:01     ` Hannes Reinecke [this message]
2008-05-14 19:38 ` James Bottomley
2008-05-14 21:50   ` James Smart
2008-05-15  1:21     ` James Smart
2008-09-24 19:13 ` Mike Christie
2008-09-24 19:17   ` Mike Christie
2008-09-25 18:40     ` Mike Christie
2008-09-25 19:03       ` James Smart
2008-09-24 19:38   ` James Smart
2008-09-25 18:15     ` Mike Christie
2008-09-26  7:46       ` Hannes Reinecke

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=482AFEE5.2090300@suse.de \
    --to=hare@suse.de \
    --cc=James.Smart@Emulex.Com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.