Re: [PATCH] libosd: Remove ignored __weak attribute
On Fri, Oct 26, 2018 at 11:15:11PM -0700, Bart Van Assche wrote: > On 10/26/18 8:35 PM, Theodore Y. Ts'o wrote: > > The second observation I'll make is that if someone is proposing a > > cleanup patch, it's unfair to dump on the person proposing the cleanup > > patch the (non-trivial) effort to drop a driver/file system/subsystem. > > Hi Ted, > > Maybe I was not clear enough. It never was my intention to suggest that Nick > or Nathan should remove the OSD code. This is something I'm willing to do > myself. BTW, I'm still waiting for someone to explain me why the patch at > the start of this thread was submitted by people who never have used the > libosd driver and who do not have any plans to use it ever. > Hi Bart, We've been cleaning up Clang warnings seen in various configurations. In this case, I believe this warning shows up in an arm64 allyesconfig build (would probably show up in an x86_64 one too but I'm not going to test right now). More info: https://github.com/ClangBuiltLinux/linux/issues/58 Cheers, Nathan > > If the maintainer wants to drop a driver/file system, that should be > > the maintainer's responsibiltiy; not someone proposing a > > cleanup/maintenance patch. > > I think anyone who makes tree-wide changes has the freedom to suggest to > remove a driver. Having to modify drivers that are no longer maintained when > doing tree-wide changes can be a real pain. > > Additionally, you may have missed earlier discussions on the linux-scsi > mailing list about this driver. The first time it was suggested to remove > this driver was several years ago. The outcome of a discussion of a few > weeks ago is that there is agreement about the removal of this driver. See > also the following messages: > * https://www.spinics.net/lists/linux-scsi/msg123738.html > * https://www.spinics.net/lists/linux-scsi/msg123742.html > > Bart.
Re: [PATCH] libosd: Remove ignored __weak attribute
On 10/26/18 8:35 PM, Theodore Y. Ts'o wrote: The second observation I'll make is that if someone is proposing a cleanup patch, it's unfair to dump on the person proposing the cleanup patch the (non-trivial) effort to drop a driver/file system/subsystem. Hi Ted, Maybe I was not clear enough. It never was my intention to suggest that Nick or Nathan should remove the OSD code. This is something I'm willing to do myself. BTW, I'm still waiting for someone to explain me why the patch at the start of this thread was submitted by people who never have used the libosd driver and who do not have any plans to use it ever. If the maintainer wants to drop a driver/file system, that should be the maintainer's responsibiltiy; not someone proposing a cleanup/maintenance patch. I think anyone who makes tree-wide changes has the freedom to suggest to remove a driver. Having to modify drivers that are no longer maintained when doing tree-wide changes can be a real pain. Additionally, you may have missed earlier discussions on the linux-scsi mailing list about this driver. The first time it was suggested to remove this driver was several years ago. The outcome of a discussion of a few weeks ago is that there is agreement about the removal of this driver. See also the following messages: * https://www.spinics.net/lists/linux-scsi/msg123738.html * https://www.spinics.net/lists/linux-scsi/msg123742.html Bart.
Re: [PATCH] libosd: Remove ignored __weak attribute
On Fri, Oct 26, 2018 at 03:07:39PM -0700, Nick Desaulniers wrote: > > That's not completely correct. The standard approach to check whether or not > > a driver is still being used is to check its git history. If the number of > > contributors is low and it was several years ago that a new feature was > > added > > or a bug has been fixed it is likely that nobody is using that driver > > anymore. > > I don't disagree with you, I just don't see how what you state can be > reconciled with Linus' response in > https://lkml.org/lkml/2018/10/27/44. Those two viewpoints seem > incompatible to me, but maybe there's a nuance I'm missing? So a couple of observations. Obviously, drivers, file systems and architectures *have* been removed. It can be done; sometimes if it can be demonstrate that it can't possibly work (for example, due to bitrot, the kernel would immediately crashed if anyone tried to use the code in question :-). In other cases, drivers has been removed through the staging subsystem, sometimes by adding a "depends on BROKEN" in the Kconfig file, and seeing if anyone complains --- since removing a "depends on BROKEN" line in Kconfig is even easier than doing reverting a git commit (especially if the user downloaded a tarball instead of doing a git clone). If you've done your due diligence then the chances that you have to revert a change which disables and later removes the dead code can be pushed close to zero. The question is whether it's worth the effort. > Nathan and I are just pointing out a small fix to eliminate a small > warning, deleting all this code does kind of feels like "throwing out > the baby with the bath water." A nuclear option for what would be a > small change otherwise. Maybe it's good to discuss the EOL for > exofs/osd, but can we please decouple that conversation from the small > change Nathan and I are proposing? The second observation I'll make is that if someone is proposing a cleanup patch, it's unfair to dump on the person proposing the cleanup patch the (non-trivial) effort to drop a driver/file system/subsystem. If the maintainer wants to drop a driver/file system, that should be the maintainer's responsibiltiy; not someone proposing a cleanup/maintenance patch. - Ted
Re: [PATCH] libosd: Remove ignored __weak attribute
On Fri, 2018-10-26 at 15:07 -0700, Nick Desaulniers wrote: > I don't disagree with you, I just don't see how what you state can be > reconciled with Linus' response in > https://lkml.org/lkml/2018/10/27/44. Those two viewpoints seem > incompatible to me, but maybe there's a nuance I'm missing? I don't think there is any disagreement nor that there are any conflicting viewpoints. As explained in previous e-mails it is unlikely that anyone is using these kernel drivers and as far as I know Linus is OK with removing unused kernel drivers. > Nathan and I are just pointing out a small fix to eliminate a small > warning, deleting all this code does kind of feels like "throwing out > the baby with the bath water." A nuclear option for what would be a > small change otherwise. Maybe it's good to discuss the EOL for > exofs/osd, but can we please decouple that conversation from the small > change Nathan and I are proposing? Removing a kernel driver is not a nuclear option. You may not be aware of this but it happens all the time. From a maintainer point of view it is a very sensible action because there are people who keep submitting cleanup patches for kernel drivers they do not use themselves. Every individual patch needs some attention and hence causes some work for a kernel maintainer. Removing kernel drivers that are not used helps to reduce the workload of a maintainer and hence is a rational action. Additionally, if anyone would ever complain about removal of a kernel driver, it can be brought back by reverting the commit through which it has been removed. Martin, please reply if you see this differently. Bart.
Re: [PATCH] libosd: Remove ignored __weak attribute
On Fri, Oct 26, 2018 at 2:59 PM Bart Van Assche wrote:
>
> On Fri, 2018-10-26 at 14:36 -0700, Nick Desaulniers wrote:
> > On Fri, Oct 26, 2018 at 2:30 PM Bart Van Assche wrote:
> > >
> > > On Fri, 2018-10-26 at 14:00 -0700, Nick Desaulniers wrote:
> > > > On Fri, Oct 26, 2018 at 11:01 AM Bart Van Assche
> > > > wrote:
> > > > >
> > > > > On Fri, 2018-10-26 at 10:54 -0700, Nick Desaulniers wrote:
> > > > > > If creating one instance of this variable is a functional change, I
> > > > > > can't help but suspect the original code was wrong. But maybe Bart,
> > > > > > Boaz, or Christoph can clarify or have more thoughts on this? Looks
> > > > > > like Boaz added this header in commit de258bf5e638 ("[SCSI] libosd:
> > > > > > OSDv1 Headers").
> > > > >
> > > > > Hi Nick and Nathan,
> > > > >
> > > > > Had you noticed the following e-mail from early October:
> > > > > https://marc.info/?l=linux-kernel&m=153849955503249?
> > > >
> > > > From this subthread with Linus, removal of the exofs fs and scsi osd
> > > > code would be a user visible change and is not an option. See:
> > > > https://lkml.org/lkml/2018/10/27/3
> > > > https://lkml.org/lkml/2018/10/27/44
> > >
> > > Hi Nick,
> > >
> > > Linus wrote that removing a filesystem is considered a userspace breakage
> > > if a user notices. The key part is "if a user notices". Who are the exofs
> > > users?
> >
> > See my thoughts on this in https://lkml.org/lkml/2018/10/27/27.
> > Particularly the part about the IMO catch 22.
> >
> > Neither you nor I can claim "there are none."
>
> That's not completely correct. The standard approach to check whether or not
> a driver is still being used is to check its git history. If the number of
> contributors is low and it was several years ago that a new feature was added
> or a bug has been fixed it is likely that nobody is using that driver anymore.
>
> Bart.
>
Bart,
I don't disagree with you, I just don't see how what you state can be
reconciled with Linus' response in
https://lkml.org/lkml/2018/10/27/44. Those two viewpoints seem
incompatible to me, but maybe there's a nuance I'm missing?
Nathan and I are just pointing out a small fix to eliminate a small
warning, deleting all this code does kind of feels like "throwing out
the baby with the bath water." A nuclear option for what would be a
small change otherwise. Maybe it's good to discuss the EOL for
exofs/osd, but can we please decouple that conversation from the small
change Nathan and I are proposing?
--
Thanks,
~Nick Desaulniers
Re: [PATCH] libosd: Remove ignored __weak attribute
On Fri, 2018-10-26 at 14:36 -0700, Nick Desaulniers wrote:
> On Fri, Oct 26, 2018 at 2:30 PM Bart Van Assche wrote:
> >
> > On Fri, 2018-10-26 at 14:00 -0700, Nick Desaulniers wrote:
> > > On Fri, Oct 26, 2018 at 11:01 AM Bart Van Assche
> > > wrote:
> > > >
> > > > On Fri, 2018-10-26 at 10:54 -0700, Nick Desaulniers wrote:
> > > > > If creating one instance of this variable is a functional change, I
> > > > > can't help but suspect the original code was wrong. But maybe Bart,
> > > > > Boaz, or Christoph can clarify or have more thoughts on this? Looks
> > > > > like Boaz added this header in commit de258bf5e638 ("[SCSI] libosd:
> > > > > OSDv1 Headers").
> > > >
> > > > Hi Nick and Nathan,
> > > >
> > > > Had you noticed the following e-mail from early October:
> > > > https://marc.info/?l=linux-kernel&m=153849955503249?
> > >
> > > From this subthread with Linus, removal of the exofs fs and scsi osd
> > > code would be a user visible change and is not an option. See:
> > > https://lkml.org/lkml/2018/10/27/3
> > > https://lkml.org/lkml/2018/10/27/44
> >
> > Hi Nick,
> >
> > Linus wrote that removing a filesystem is considered a userspace breakage
> > if a user notices. The key part is "if a user notices". Who are the exofs
> > users?
>
> See my thoughts on this in https://lkml.org/lkml/2018/10/27/27.
> Particularly the part about the IMO catch 22.
>
> Neither you nor I can claim "there are none."
That's not completely correct. The standard approach to check whether or not
a driver is still being used is to check its git history. If the number of
contributors is low and it was several years ago that a new feature was added
or a bug has been fixed it is likely that nobody is using that driver anymore.
Bart.
Re: [PATCH] libosd: Remove ignored __weak attribute
On Fri, Oct 26, 2018 at 2:30 PM Bart Van Assche wrote:
>
> On Fri, 2018-10-26 at 14:00 -0700, Nick Desaulniers wrote:
> > On Fri, Oct 26, 2018 at 11:01 AM Bart Van Assche wrote:
> > >
> > > On Fri, 2018-10-26 at 10:54 -0700, Nick Desaulniers wrote:
> > > > If creating one instance of this variable is a functional change, I
> > > > can't help but suspect the original code was wrong. But maybe Bart,
> > > > Boaz, or Christoph can clarify or have more thoughts on this? Looks
> > > > like Boaz added this header in commit de258bf5e638 ("[SCSI] libosd:
> > > > OSDv1 Headers").
> > >
> > > Hi Nick and Nathan,
> > >
> > > Had you noticed the following e-mail from early October:
> > > https://marc.info/?l=linux-kernel&m=153849955503249?
> >
> > From this subthread with Linus, removal of the exofs fs and scsi osd
> > code would be a user visible change and is not an option. See:
> > https://lkml.org/lkml/2018/10/27/3
> > https://lkml.org/lkml/2018/10/27/44
>
> Hi Nick,
>
> Linus wrote that removing a filesystem is considered a userspace breakage
> if a user notices. The key part is "if a user notices". Who are the exofs
> users?
See my thoughts on this in https://lkml.org/lkml/2018/10/27/27.
Particularly the part about the IMO catch 22.
Neither you nor I can claim "there are none."
--
Thanks,
~Nick Desaulniers
Re: [PATCH] libosd: Remove ignored __weak attribute
On Fri, 2018-10-26 at 14:00 -0700, Nick Desaulniers wrote:
> On Fri, Oct 26, 2018 at 11:01 AM Bart Van Assche wrote:
> >
> > On Fri, 2018-10-26 at 10:54 -0700, Nick Desaulniers wrote:
> > > If creating one instance of this variable is a functional change, I
> > > can't help but suspect the original code was wrong. But maybe Bart,
> > > Boaz, or Christoph can clarify or have more thoughts on this? Looks
> > > like Boaz added this header in commit de258bf5e638 ("[SCSI] libosd:
> > > OSDv1 Headers").
> >
> > Hi Nick and Nathan,
> >
> > Had you noticed the following e-mail from early October:
> > https://marc.info/?l=linux-kernel&m=153849955503249?
>
> From this subthread with Linus, removal of the exofs fs and scsi osd
> code would be a user visible change and is not an option. See:
> https://lkml.org/lkml/2018/10/27/3
> https://lkml.org/lkml/2018/10/27/44
Hi Nick,
Linus wrote that removing a filesystem is considered a userspace breakage
if a user notices. The key part is "if a user notices". Who are the exofs
users?
Bart.
Re: [PATCH] libosd: Remove ignored __weak attribute
On Fri, Oct 26, 2018 at 1:42 PM Linus Torvalds wrote: > > On Fri, Oct 26, 2018 at 1:06 PM Nick Desaulniers > wrote: > > > > Is removing a filesystem considered a userspace breakage? > > Yes - if a user notices. > > The key word is *USER*. > > Note that it's not "user space". It's not about _programs_ noticing, > it's literally about users and their workflows. > > If some change breaks a real user workflow, it needs to be reverted. > > So this is not about ABI or anything like that. We've had cases where > the ABI stayed the same, but the order of device probing changed, and > that broke peoples setups (because now /dev/sdb and /dev/sda switched > places), and we had to revert. > > It's literally about "if a user upgrades a kernel, and something no > longer works, it's a regression". > > In general, a good idea is "if you have to wonder about it, just don't > do it". Because it turns out that users are odd, and often do odd > things much after you'd have thought they'd have long since switched > to more modern hardware or filesystems. > > Linus Makes sense and is a consistent stance. Thanks for clarifying. Will pursue the smaller fix in the other subthread. https://lkml.org/lkml/2018/10/27/55 -- Thanks, ~Nick Desaulniers
Re: [PATCH] libosd: Remove ignored __weak attribute
On Fri, Oct 26, 2018 at 11:01 AM Bart Van Assche wrote:
>
> On Fri, 2018-10-26 at 10:54 -0700, Nick Desaulniers wrote:
> > If creating one instance of this variable is a functional change, I
> > can't help but suspect the original code was wrong. But maybe Bart,
> > Boaz, or Christoph can clarify or have more thoughts on this? Looks
> > like Boaz added this header in commit de258bf5e638 ("[SCSI] libosd:
> > OSDv1 Headers").
>
> Hi Nick and Nathan,
>
> Had you noticed the following e-mail from early October:
> https://marc.info/?l=linux-kernel&m=153849955503249?
>From this subthread with Linus, removal of the exofs fs and scsi osd
code would be a user visible change and is not an option. See:
https://lkml.org/lkml/2018/10/27/3
https://lkml.org/lkml/2018/10/27/44
So we should pursue this much smaller fixup. Boaz, can you clarify
https://lkml.org/lkml/2018/10/26/682
specifically:
> If creating one instance of this variable is a functional change, I
> can't help but suspect the original code was wrong. But maybe Bart,
> Boaz, or Christoph can clarify or have more thoughts on this? Looks
> like Boaz added this header in commit de258bf5e638 ("[SCSI] libosd:
> OSDv1 Headers").
--
Thanks,
~Nick Desaulniers
Re: [PATCH] libosd: Remove ignored __weak attribute
On Fri, Oct 26, 2018 at 1:06 PM Nick Desaulniers wrote: > > Is removing a filesystem considered a userspace breakage? Yes - if a user notices. The key word is *USER*. Note that it's not "user space". It's not about _programs_ noticing, it's literally about users and their workflows. If some change breaks a real user workflow, it needs to be reverted. So this is not about ABI or anything like that. We've had cases where the ABI stayed the same, but the order of device probing changed, and that broke peoples setups (because now /dev/sdb and /dev/sda switched places), and we had to revert. It's literally about "if a user upgrades a kernel, and something no longer works, it's a regression". In general, a good idea is "if you have to wonder about it, just don't do it". Because it turns out that users are odd, and often do odd things much after you'd have thought they'd have long since switched to more modern hardware or filesystems. Linus
Re: [PATCH] libosd: Remove ignored __weak attribute
On Fri, Oct 26, 2018 at 12:22 PM Linus Torvalds wrote: > > On Fri, Oct 26, 2018 at 11:32 AM Nick Desaulniers > wrote: > > > > + Linus > > I'm not about the process of removing code from the kernel. Doesn't > > that violate the "thou shalt not break userspace" rule? > > The only thing that breaks the "thou shalt not break userspace" rule > is fairly simple: things that break user space. Is removing a filesystem considered a userspace breakage? If you have an exofs mount, and you upgrade to a kernel with the RFC patch, you will no longer be able to mount that type of FS. Userspace code that expects to mount an exofs FS would now always fail. Maybe this is equivalent to removing a previously supported mount option. /proc/filesystems might have one less option. So I guess this is different than dropping support for an architecture since you cannot produce an updated kernel image, as opposed to this RFC which would make existing drives fail to mount? > > Does removing the code break for somebody? If so we don't do it. But > if nobody notices because nobody uses, it's fine. Right, but it seems like a catch 22; it's not possible to prove that nobody will notice because they are not using it. Like the expression: If a tree falls in a forest and no one is around to hear it, does it make a sound? If no one is using a kernel feature and its removed, will anyone notice? If the maintainer would like to remove support for their subsystem, should it continue to remain in the tree? Code is never truly deleted, and is relatively painless to resurrect with git. Solutions that come to mind immediately: * Just land the small fix discussed earlier in the thread. These subsystems continue to exist. Maybe this question comes up again for this subsystem in a few years. Christoph alludes to my question not being the first time this was asked of this subsystem. * Put out some kind of intent to deprecate, hoping to get feedback on possible users that would be affected. Land the deletion patches in -next. This approach always runs the risk of not posting in the right venue; and users can't be bothered to check for intent to deprecate notices. * Just delete it. Resurrect if users report use of this FS. What are your thoughts? > > Basically, there is no "theoretical" rule about what breaks user space > or not. In particular, the rule is *not* that you can't change ABI. > You can do any change you want that changes a kernel exported ABI, > just as long as nobody actually notices the change. > > But in practice, it's often _much_ more work to try to figure out > whether something breaks somebody than it is to just say "don't change > behavior", so 99% of the time, the rule ends up being just "try to > avoid intentionally changing behavior, because you'll likely get it > wrong". I agree semantic changes to an existing API are problematic (not just within the kernel), but do you consider deletion or removal of parameters in the same category? Thank you for your insight on this. -- Thanks, ~Nick Desaulniers
Re: [PATCH] libosd: Remove ignored __weak attribute
On Fri, Oct 26, 2018 at 11:32 AM Nick Desaulniers wrote: > > + Linus > I'm not about the process of removing code from the kernel. Doesn't > that violate the "thou shalt not break userspace" rule? The only thing that breaks the "thou shalt not break userspace" rule is fairly simple: things that break user space. Does removing the code break for somebody? If so we don't do it. But if nobody notices because nobody uses, it's fine. Basically, there is no "theoretical" rule about what breaks user space or not. In particular, the rule is *not* that you can't change ABI. You can do any change you want that changes a kernel exported ABI, just as long as nobody actually notices the change. But in practice, it's often _much_ more work to try to figure out whether something breaks somebody than it is to just say "don't change behavior", so 99% of the time, the rule ends up being just "try to avoid intentionally changing behavior, because you'll likely get it wrong". Linus
Re: [PATCH] libosd: Remove ignored __weak attribute
On Fri, Oct 26, 2018 at 11:05 AM Nathan Chancellor
wrote:
>
> On Fri, Oct 26, 2018 at 11:01:48AM -0700, Bart Van Assche wrote:
> > On Fri, 2018-10-26 at 10:54 -0700, Nick Desaulniers wrote:
> > > If creating one instance of this variable is a functional change, I
> > > can't help but suspect the original code was wrong. But maybe Bart,
> > > Boaz, or Christoph can clarify or have more thoughts on this? Looks
> > > like Boaz added this header in commit de258bf5e638 ("[SCSI] libosd:
> > > OSDv1 Headers").
> >
> > Hi Nick and Nathan,
> >
> > Had you noticed the following e-mail from early October:
> > https://marc.info/?l=linux-kernel&m=153849955503249?
> >
> > Thanks,
> >
> > Bart.
>
> Hi Bart,
>
> I had but there doesn't seem to be any reply to it so I wasn't sure if
> there was a final consensus on removing the driver. If that's the route
> that everyone wants to go, then I guess we don't need to continue to
> debate this patch.
>
> Thanks for the heads up,
> Nathan
I've staged a RFC patch here:
https://github.com/ClangBuiltLinux/linux/commit/03817d982606e2db143d23a8a55e97de6de6e0c2
+ Linus
I'm not about the process of removing code from the kernel. Doesn't
that violate the "thou shalt not break userspace" rule? But code does
get deleted from the kernel... ex:
http://lkml.iu.edu/hypermail/linux/kernel/1804.1/06654.html
--
Thanks,
~Nick Desaulniers
Re: [PATCH] libosd: Remove ignored __weak attribute
On Fri, Oct 26, 2018 at 11:01:48AM -0700, Bart Van Assche wrote:
> On Fri, 2018-10-26 at 10:54 -0700, Nick Desaulniers wrote:
> > If creating one instance of this variable is a functional change, I
> > can't help but suspect the original code was wrong. But maybe Bart,
> > Boaz, or Christoph can clarify or have more thoughts on this? Looks
> > like Boaz added this header in commit de258bf5e638 ("[SCSI] libosd:
> > OSDv1 Headers").
>
> Hi Nick and Nathan,
>
> Had you noticed the following e-mail from early October:
> https://marc.info/?l=linux-kernel&m=153849955503249?
>
> Thanks,
>
> Bart.
Hi Bart,
I had but there doesn't seem to be any reply to it so I wasn't sure if
there was a final consensus on removing the driver. If that's the route
that everyone wants to go, then I guess we don't need to continue to
debate this patch.
Thanks for the heads up,
Nathan
Re: [PATCH] libosd: Remove ignored __weak attribute
On Fri, 2018-10-26 at 10:54 -0700, Nick Desaulniers wrote:
> If creating one instance of this variable is a functional change, I
> can't help but suspect the original code was wrong. But maybe Bart,
> Boaz, or Christoph can clarify or have more thoughts on this? Looks
> like Boaz added this header in commit de258bf5e638 ("[SCSI] libosd:
> OSDv1 Headers").
Hi Nick and Nathan,
Had you noticed the following e-mail from early October:
https://marc.info/?l=linux-kernel&m=153849955503249?
Thanks,
Bart.
Re: [PATCH v3 8/8] sg: user controls for q_at_head, read_value
On 2018-10-26 12:48 p.m., Douglas Gilbert wrote: Add a SG_SET_GET_EXTENDED ioctl control for whether commands will be queued_at_head or queued_at_tail by the block layer (together with the scsi mid-level). It has file scope. Also add a read_value integer the can be used by write a value from the SG_SEIRV_* group then the corresponding value will be returned. Signed-off-by: Douglas Gilbert --- See "v3.5" of this patch to fix the compile error when CONFIG_SCSI_PROC_FS is not defined in a kernel build. Doug Gilbert
[PATCH v3.5 8/8] sg: user controls for q_at_head, read_value
Add a SG_SET_GET_EXTENDED ioctl control for whether commands
will be queued_at_head or queued_at_tail by the block layer
(together with the scsi mid-level). It has file scope.
Also add a read_value integer the can be used by write a
value from the SG_SEIRV_* group then the corresponding value
will be returned.
Signed-off-by: Douglas Gilbert
---
The user can still override the new file scope setting on a
a per command basis with the SG_FLAG_Q_AT_HEAD and
SG_FLAG_Q_AT_TAIL in the sg v3 and v4 structures.
An example of read_value usage is to write the value
SG_SEIRV_FL_RQS to the read_value field. Then after the
SG_SET_GET_EXTENDED ioctl is run, the number of (inactive)
requests currently on this file descriptor's request free
list is placed in the read_value field.
Added in v3 is SG_SEIRV_DEV_FL_RQS which is an expansion of
SG_SEIRV_FL_RQS. SG_SEIRV_DEV_FL_RQS counts free list entries
on all sg file descriptors currently open on the device that
the file descriptor (given to ioctl()) is associated with.
Change since v3 [patches 1 to 7 of 8 are unchanged, this for 8/8]:
- move sg_version_date definition from inside CONFIG_SCSI_PROC_FS
conditional out to full file scope and near related definitions
drivers/scsi/sg.c | 160 +++--
include/scsi/sg.h | 32 ++---
include/uapi/scsi/sg.h | 49 ++---
3 files changed, 150 insertions(+), 91 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 3e89bbd508de..5ba92f112481 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -18,6 +18,7 @@
static int sg_version_num = 30901; /* 2 digits for each component */
#define SG_VERSION_STR "3.9.01"
+static char *sg_version_date = "20181024";
#include
@@ -59,7 +60,6 @@ static int sg_version_num = 30901;/* 2 digits for each
component */
#ifdef CONFIG_SCSI_PROC_FS
#include
-static char *sg_version_date = "20181019";
static int sg_proc_init(void);
#endif
@@ -95,6 +95,10 @@ enum sg_rq_state {
#define SG_DEF_TIME_UNIT SG_TIME_UNIT_MS
#define SG_DEFAULT_TIMEOUT mult_frac(SG_DEFAULT_TIMEOUT_USER, HZ, USER_HZ)
+#define SG_FD_Q_AT_TAIL true
+#define SG_FD_Q_AT_HEAD false
+#define SG_DEFAULT_Q_AT SG_FD_Q_AT_HEAD/* for backward compatibility */
+
int sg_big_buff = SG_DEF_RESERVED_SIZE;
/* N.B. This variable is readable and writeable via
/proc/scsi/sg/def_reserved_size . Each time sg_open() is called a buffer
@@ -187,6 +191,7 @@ struct sg_fd { /* holds the state of a file
descriptor */
bool keep_orphan;/* false -> drop (def), true -> keep for read() */
bool mmap_called; /* false -> mmap() never called on this fd */
bool time_in_ns;/* report times in nanoseconds */
+ bool q_at_tail; /* queue at tail if true, head when false */
u8 next_cmd_len;/* 0: automatic, >0: use on next write() */
struct sg_request *reserve_srp; /* allocate on open(), starts on fl */
struct fasync_struct *async_qp; /* used by asynchronous notification */
@@ -238,7 +243,7 @@ static struct sg_request *sg_add_request(struct sg_fd *sfp,
int dxfr_len,
static void sg_remove_request(struct sg_fd *sfp, struct sg_request *srp);
static struct sg_device *sg_get_dev(int min_dev);
static void sg_device_destroy(struct kref *kref);
-static const char *sg_rq_state_str(u8 rq_state, bool long_str);
+static const char *sg_rq_state_str(enum sg_rq_state rq_state, bool long_str);
static struct sg_request *sg_mk_srp(struct sg_fd *sfp, bool first,
rwlock_t *rwlp, unsigned long *iflagsp);
@@ -855,7 +860,7 @@ sg_common_write(struct sg_fd *sfp, const struct sg_io_hdr
*hi_p,
if (h4p || !hi_p)
return ERR_PTR(-EOPNOTSUPP);
- srp = sg_add_request(sfp, hi_p->dxfer_len, false);
+ srp = sg_add_request(sfp, hi_p->dxfer_len, sync);
if (IS_ERR(srp))
return srp;
srp->header = *hi_p;/* structure assignment, could memcpy */
@@ -897,9 +902,13 @@ sg_common_write(struct sg_fd *sfp, const struct sg_io_hdr
*hi_p,
srp->start_ts = ktime_get_with_offset(TK_OFFS_BOOT);
else
hp->duration = jiffies_to_msecs(jiffies);
- /* at tail if v3 or later interface and tail flag set */
- at_head = !(hp->interface_id != '\0' &&
- (SG_FLAG_Q_AT_TAIL & hp->flags));
+
+ if (hp->interface_id == '\0') /* v1 and v2 interface */
+ at_head = true; /* backward compatibility */
+ else if (sfp->q_at_tail) /* cmd flags can override sfd setting */
+ at_head = (hp->flags & SG_FLAG_Q_AT_HEAD);
+ else/* this sfd is defaulting to head */
+ at_head = !(hp->flags & SG_FLAG_Q_AT_TAIL);
srp->rq->timeout = timeout;
kref_get(&sfp->f_ref); /* sg_rq_end_io() does kref_put(). */
@@ -1084,30 +1093,30 @@ sg_reserved_sz(struct sg_fd *sfp, struc
Re: [PATCH] libosd: Remove ignored __weak attribute
On Thu, Oct 25, 2018 at 3:55 PM Nathan Chancellor
wrote:
>
> On Thu, Oct 25, 2018 at 03:02:13PM -0700, Nick Desaulniers wrote:
> > On Thu, Oct 25, 2018 at 2:31 PM Nathan Chancellor
> > wrote:
> > >
> > > On Tue, Oct 02, 2018 at 04:06:31PM -0700, Bart Van Assche wrote:
> > > > On Tue, 2018-10-02 at 15:33 -0700, Nick Desaulniers wrote:
> > > > > On Tue, Oct 2, 2018 at 10:57 AM Bart Van Assche
> > > > > wrote:
> > > > > > Explicitly initialized global and static variables end up in the
> > > > > > .data
> > > > > > section and need space in that section.
> > > > >
> > > > > Unless the initial value is zero.
> > > > > https://godbolt.org/z/curRoO
> > > > >
> > > > > So you don't wind up with an increase in binary size simply by having
> > > > > global variables initialized to zero, right? Instead the kernel knows
> > > > > to create a zero'd out mapping for bss. You don't need a run of zeros
> > > > > in the binary.
> > > > >
> > > > > So I disagree when you said earlier "zero initializers should be left
> > > > > out to minimize the size of object files." I assert they don't affect
> > > > > the size of the binary.
> > > > >
> > > > > If you had many global variables all initialized to zero, why would
> > > > > you encode that many zeros in a binary, when you can just set a size
> > > > > on the bss section and have the kernel create the appropriate sized
> > > > > and zero'd mapping?
> > > > >
> > > > > > That is not the case if the
> > > > > > initializer is left out and these variables end up in the .bss
> > > > > > section.
> > > > >
> > > > > From my above link, gcc will put globals without initializers into
> > > > > "common."
> > > >
> > > > No matter what particular compiler versions do with explicit
> > > > initialization
> > > > to zero, the preferred kernel coding style is to leave out such explicit
> > > > initialization.
> > > >
> > > > Bart.
> > >
> > > Hi Bart,
> > >
> > > I'm sorry if I didn't follow the conclusion of this conversation properly
> > > but this is the below diff you were initially looking for, correct?
> > >
> > > If so, Boaz and Nick, do you have any objections if this is v2? I'd like
> > > to get this patch accepted so the warning can be fixed for everyone.
> >
> > Hi Nathan,
> > Thanks for following up on this. Bart's note about the one definition
> > rule is important. If you define the variable static in two different
> > translation units, you've suddenly created two different copies
> > accessible only to their respective translation units. So it should
> > be declared extern in one source file (but not defined/initialized),
> > and defined (non-static) in another. See below for example.
> >
>
> Hi Nick,
>
> I just want to make sure I understand what is going on here.
>
> Doesn't the first part already happen because osd_root_object is
> declared static in osd_types.h? I tried this little simple example of
> adding a 'static const' variable to a header file and using it in two
> separate files/functions. When compiled together, they point to two
> different locations in memory.
>
> ==
>
> $ clang -std=gnu89 main.c test1.c test2.c
> $ ./a.out
> test in test1(): 0x55b4df3a001c
> test in test2(): 0x55b4df3a003c
>
> ==
>
> main.c:
>
> #include "test.h"
>
> int main(void) {
> test1();
> test2();
> }
>
> ==
>
> test1.c:
>
> #include
> #include "test.h"
>
> void test1() {
> printf("test in test1(): %p\n", &test);
> }
>
> ==
>
> test2.c:
>
> #include
> #include "test.h"
>
> void test2() {
> printf("test in test2(): %p\n", &test);
> }
>
> ==
>
> test.h:
>
> struct test_struct {
> int a;
> int b;
> };
>
> static const struct test_struct test = {0, 0};
> void test1();
> void test2();
>
> ==
>
> If that is the case, could your suggested change result in a functional
> change given that the code would now refer to the same osd_root_object?
It's hard to say without knowing the original intent of the code.
>From the variable's identifier and fact that it's global, I *assume*
that we want only 1 struct osd_obj_id which is the root, hence the
identifier `osd_root_object`. It has 4 references in the entire
kernel; it doesn't make sense to my why those references would want to
be referring to two different instances of `osd_root_object`. Maybe
the maintainers can clarify if 2 instances is the intent?
Further complicated is the use of the __weak attribute AND the
compiler flag -fno-common (which the kernel sets in the top level
Makefile). Also, it seems that ODR is a C++ concept; it's not clear
to me if there's semantic differences with C or not (I don't think so
in this case, but I've learned not to bet on subtle semantic
differences between the languages not existing).
__attrib
Re: [PATCH v3 8/8] sg: user controls for q_at_head, read_value
Hi linux-scsi-owner,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on mkp-scsi/for-next]
[also build test ERROR on v4.19 next-20181019]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/linux-scsi-owner-vger-kernel-org/sg-types-and-naming-cleanup/20181026-220008
base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: x86_64-randconfig-r0-10262157 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
In file included from include/linux/kernel.h:14:0,
from include/linux/list.h:9,
from include/linux/module.h:9,
from drivers/scsi/sg.c:22:
drivers/scsi/sg.c: In function 'init_sg':
>> drivers/scsi/sg.c:2277:3: error: 'sg_version_date' undeclared (first use in
>> this function)
sg_version_date);
^
include/linux/printk.h:315:34: note: in definition of macro 'pr_info'
printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
^~~
drivers/scsi/sg.c:2277:3: note: each undeclared identifier is reported only
once for each function it appears in
sg_version_date);
^
include/linux/printk.h:315:34: note: in definition of macro 'pr_info'
printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
^~~
drivers/scsi/sg.c: At top level:
drivers/scsi/sg.c:246:20: warning: 'sg_rq_state_str' used but never defined
static const char *sg_rq_state_str(enum sg_rq_state rq_state, bool
long_str);
^~~
vim +/sg_version_date +2277 drivers/scsi/sg.c
2256
2257 static int __init
2258 init_sg(void)
2259 {
2260 int rc;
2261
2262 if (scatter_elem_sz < PAGE_SIZE) {
2263 scatter_elem_sz = PAGE_SIZE;
2264 scatter_elem_sz_prev = scatter_elem_sz;
2265 }
2266 if (def_reserved_size >= 0)
2267 sg_big_buff = def_reserved_size;
2268 else
2269 def_reserved_size = sg_big_buff;
2270
2271 rc = register_chrdev_region(MKDEV(SCSI_GENERIC_MAJOR, 0),
2272 SG_MAX_DEVS, "sg");
2273 if (rc)
2274 return rc;
2275 pr_info("Registered %s[char major=0x%x], version: %s, date:
%s\n",
2276 "sg device ", SCSI_GENERIC_MAJOR, SG_VERSION_STR,
> 2277 sg_version_date);
2278 sg_sysfs_class = class_create(THIS_MODULE, "scsi_generic");
2279 if ( IS_ERR(sg_sysfs_class) ) {
2280 rc = PTR_ERR(sg_sysfs_class);
2281 goto err_out;
2282 }
2283 sg_sysfs_valid = 1;
2284 rc = scsi_register_interface(&sg_interface);
2285 if (0 == rc) {
2286 #ifdef CONFIG_SCSI_PROC_FS
2287 sg_proc_init();
2288 #endif /* CONFIG_SCSI_PROC_FS */
2289 return 0;
2290 }
2291 class_destroy(sg_sysfs_class);
2292 err_out:
2293 unregister_chrdev_region(MKDEV(SCSI_GENERIC_MAJOR, 0),
SG_MAX_DEVS);
2294 return rc;
2295 }
2296
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
Re: [PATCH v3 2/5] qla2xxx_nvmet: Add files for FC-NVMe Target support
Hi James, > On Oct 25, 2018, at 11:23 AM, James Smart wrote: > > External Email > > On 9/28/2018 3:46 PM, Himanshu Madhani wrote: >> + .target_features= NVMET_FCTGTFEAT_READDATA_RSP | >> + NVMET_FCTGTFEAT_CMD_IN_ISR | >> + NVMET_FCTGTFEAT_OPDONE_IN_ISR, >> > > Himanshu, > > I'm looking at these but had a quick question. Did you really want the > IN_ISR flags set ? they schedule processing vs calling the nvmet > routines inline. The intent was the queueing was only needed if in the > hard isr routine. Last contact I had with your group said you were in > soft isr routines and inline calling would be used. I'm asking because > I had intended to remove these flags/features. > > -- james > Looks like there was a miss to remove these flags when we rebased code on 4.20/scsi-queue. After the original submission where this flag was present, we have removed this flag in our internal testing but the code sent out after rebase missed that update. I’ll send v4 with the flag removed. Please let me know you have any other comments that I can incorporate in v4 Thanks, - Himanshu
Re: [PATCH] bsg: convert to use blk-mq
On Fri, Oct 26, 2018 at 10:08:30AM -0600, Jens Axboe wrote:
> On 10/26/18 10:06 AM, Benjamin Block wrote:
> > On Thu, Oct 25, 2018 at 05:12:59PM -0600, Jens Axboe wrote:
> >> On 10/23/18 12:07 PM, Jens Axboe wrote:
> >>> On 10/23/18 11:40 AM, Benjamin Block wrote:
>
> So I tried 4.19.0 with only the two patches from you:
> http://git.kernel.dk/cgit/linux-block/commit/?h=mq-conversions&id=2b2ffa16193e9a69a076595ed64429b8cc9b42aa
> http://git.kernel.dk/cgit/linux-block/commit/?h=mq-conversions&id=142dc9f36e3113b6a76d472978c33c8c2a2b702c
>
> This fixed the first warning from before, as you suggested, but it still
> crash like this:
>
> [ ] Unable to handle kernel pointer dereference in virtual kernel
> address space
> [ ] Failing address: TEID: 0483
> [ ] Fault in home space mode while using kernel ASCE.
> [ ] AS:025f0007 R3:dffb8007 S:dffbf000
> P:013d
> [ ] Oops: 0004 ilc:3 [#1] PREEMPT SMP DEBUG_PAGEALLOC
> [ ] Modules linked in:
> [ ] CPU: 2 PID: 609 Comm: kworker/2:1H Kdump: loaded Tainted: GW
> 4.19.0-bb-next+ #1
> [ ] Hardware name: IBM 3906 M03 704 (LPAR)
> [ ] Workqueue: kblockd blk_mq_run_work_fn
> [ ] Krnl PSW : 0704e0018000 03ff806a6b40
> (zfcp_fc_exec_bsg_job+0x1c0/0x440 [zfcp])
> [ ]R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:2 PM:0 RI:0
> EA:3
> [ ] Krnl GPRS: 83e0f3c0
> 0300
> [ ]0300 03ff806a6b3a a86b5948
> a86b5988
> [ ]83e0f3f0 a86b5938
> 984aee80
> [ ]a86b5800 03ff806ba950 03ff806a6b3a
> 98a5ed88
> [ ] Krnl Code: 03ff806a6b30: b9040029lgr %r2,%r9
> 03ff806a6b34: c0e58b7ebrasl
> %r14,3ff80698230
> #03ff806a6b3a: e310f0a4lg
> %r1,160(%r15)
> >03ff806a6b40: e3109024stg %r1,0(%r9)
> 03ff806a6b46: 4120a040la %r2,64(%r10)
> 03ff806a6b4a: c0e58ae7brasl
> %r14,3ff80698118
> 03ff806a6b50: e310a044lg %r1,64(%r10)
> 03ff806a6b56: e310f0a00024stg
> %r1,160(%r15)
> [ ] Call Trace:
> [ ] ([<03ff806a6aa2>] zfcp_fc_exec_bsg_job+0x122/0x440 [zfcp])
> [ ] [<03ff8061e928>] fc_bsg_dispatch+0x310/0x398 [scsi_transport_fc]
> [ ] [<00d2995a>] bsg_queue_rq+0x15a/0x198
> [ ] [<00d03566>] blk_mq_dispatch_rq_list+0x966/0xf90
> [ ] [<00d0e37a>] blk_mq_sched_dispatch_requests+0x3fa/0x410
> [ ] [<00cfc230>] __blk_mq_run_hw_queue+0x218/0x248
> [ ] [<001cb124>] process_one_work+0x8c4/0xe90
> [ ] [<001cbe58>] worker_thread+0x768/0xbb0
> [ ] [<001dc67a>] kthread+0x22a/0x248
> [ ] [<0137b812>] kernel_thread_starter+0x6/0xc
> [ ] [<0137b80c>] kernel_thread_starter+0x0/0xc
> [ ] INFO: lockdep is turned off.
> [ ] Last Breaking-Event-Address:
> [ ] [<005d9b08>] __asan_store8+0x98/0xa0
> [ ]
> [ ] Kernel panic - not syncing: Fatal exception: panic_on_oops
>
> zfcp_fc_exec_bsg_job+0x1c0 is here:
>
> int zfcp_fc_exec_bsg_job(struct bsg_job *job)
> {
> struct Scsi_Host *shost;
> struct zfcp_adapter *adapter;
> struct zfcp_fsf_ct_els *ct_els = job->dd_data;
> struct fc_bsg_request *bsg_request = job->request;
> struct fc_rport *rport = fc_bsg_to_rport(job);
>
> shost = rport ? rport_to_shost(rport) : fc_bsg_to_shost(job);
> adapter = (struct zfcp_adapter *)shost->hostdata[0];
>
> if (!(atomic_read(&adapter->status) &
> ZFCP_STATUS_COMMON_OPEN))
> return -EINVAL;
>
> ==> ct_els->req = job->request_payload.sg_list;
> ct_els->resp = job->reply_payload.sg_list;
> ct_els->handler_data = job;
>
> switch (bsg_request->msgcode) {
> case FC_BSG_RPT_ELS:
> case FC_BSG_HST_ELS_NOLOGIN:
> return zfcp_fc_exec_els_job(job, adapter);
> case FC_BSG_RPT_CT:
> case FC_BSG_HST_CT:
> return zfcp_fc_exec_ct_job(job, adapter);
> default:
> return -EINVAL;
> }
> }
>
> I took a dump to find out how struct bsg_job looks like when it cra
Re: [PATCH] bsg: convert to use blk-mq
On 10/26/18 10:06 AM, Benjamin Block wrote:
> On Thu, Oct 25, 2018 at 05:12:59PM -0600, Jens Axboe wrote:
>> On 10/23/18 12:07 PM, Jens Axboe wrote:
>>> On 10/23/18 11:40 AM, Benjamin Block wrote:
So I tried 4.19.0 with only the two patches from you:
http://git.kernel.dk/cgit/linux-block/commit/?h=mq-conversions&id=2b2ffa16193e9a69a076595ed64429b8cc9b42aa
http://git.kernel.dk/cgit/linux-block/commit/?h=mq-conversions&id=142dc9f36e3113b6a76d472978c33c8c2a2b702c
This fixed the first warning from before, as you suggested, but it still
crash like this:
[ ] Unable to handle kernel pointer dereference in virtual kernel address
space
[ ] Failing address: TEID: 0483
[ ] Fault in home space mode while using kernel ASCE.
[ ] AS:025f0007 R3:dffb8007 S:dffbf000
P:013d
[ ] Oops: 0004 ilc:3 [#1] PREEMPT SMP DEBUG_PAGEALLOC
[ ] Modules linked in:
[ ] CPU: 2 PID: 609 Comm: kworker/2:1H Kdump: loaded Tainted: GW
4.19.0-bb-next+ #1
[ ] Hardware name: IBM 3906 M03 704 (LPAR)
[ ] Workqueue: kblockd blk_mq_run_work_fn
[ ] Krnl PSW : 0704e0018000 03ff806a6b40
(zfcp_fc_exec_bsg_job+0x1c0/0x440 [zfcp])
[ ]R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:2 PM:0 RI:0 EA:3
[ ] Krnl GPRS: 83e0f3c0
0300
[ ]0300 03ff806a6b3a a86b5948
a86b5988
[ ]83e0f3f0 a86b5938
984aee80
[ ]a86b5800 03ff806ba950 03ff806a6b3a
98a5ed88
[ ] Krnl Code: 03ff806a6b30: b9040029lgr %r2,%r9
03ff806a6b34: c0e58b7ebrasl
%r14,3ff80698230
#03ff806a6b3a: e310f0a4lg %r1,160(%r15)
>03ff806a6b40: e3109024stg %r1,0(%r9)
03ff806a6b46: 4120a040la %r2,64(%r10)
03ff806a6b4a: c0e58ae7brasl
%r14,3ff80698118
03ff806a6b50: e310a044lg %r1,64(%r10)
03ff806a6b56: e310f0a00024stg %r1,160(%r15)
[ ] Call Trace:
[ ] ([<03ff806a6aa2>] zfcp_fc_exec_bsg_job+0x122/0x440 [zfcp])
[ ] [<03ff8061e928>] fc_bsg_dispatch+0x310/0x398 [scsi_transport_fc]
[ ] [<00d2995a>] bsg_queue_rq+0x15a/0x198
[ ] [<00d03566>] blk_mq_dispatch_rq_list+0x966/0xf90
[ ] [<00d0e37a>] blk_mq_sched_dispatch_requests+0x3fa/0x410
[ ] [<00cfc230>] __blk_mq_run_hw_queue+0x218/0x248
[ ] [<001cb124>] process_one_work+0x8c4/0xe90
[ ] [<001cbe58>] worker_thread+0x768/0xbb0
[ ] [<001dc67a>] kthread+0x22a/0x248
[ ] [<0137b812>] kernel_thread_starter+0x6/0xc
[ ] [<0137b80c>] kernel_thread_starter+0x0/0xc
[ ] INFO: lockdep is turned off.
[ ] Last Breaking-Event-Address:
[ ] [<005d9b08>] __asan_store8+0x98/0xa0
[ ]
[ ] Kernel panic - not syncing: Fatal exception: panic_on_oops
zfcp_fc_exec_bsg_job+0x1c0 is here:
int zfcp_fc_exec_bsg_job(struct bsg_job *job)
{
struct Scsi_Host *shost;
struct zfcp_adapter *adapter;
struct zfcp_fsf_ct_els *ct_els = job->dd_data;
struct fc_bsg_request *bsg_request = job->request;
struct fc_rport *rport = fc_bsg_to_rport(job);
shost = rport ? rport_to_shost(rport) : fc_bsg_to_shost(job);
adapter = (struct zfcp_adapter *)shost->hostdata[0];
if (!(atomic_read(&adapter->status) & ZFCP_STATUS_COMMON_OPEN))
return -EINVAL;
==> ct_els->req = job->request_payload.sg_list;
ct_els->resp = job->reply_payload.sg_list;
ct_els->handler_data = job;
switch (bsg_request->msgcode) {
case FC_BSG_RPT_ELS:
case FC_BSG_HST_ELS_NOLOGIN:
return zfcp_fc_exec_els_job(job, adapter);
case FC_BSG_RPT_CT:
case FC_BSG_HST_CT:
return zfcp_fc_exec_ct_job(job, adapter);
default:
return -EINVAL;
}
}
I took a dump to find out how struct bsg_job looks like when it crashes
(register clobbering isn't as bad here and I verified that job->dev is
valid).
crash> print *((struct bsg_job *)0xa86b5938)
$5 = {
dev = 0x9af10358,
kref = {
refcount = {
refs = {
Re: [PATCH] bsg: convert to use blk-mq
On Thu, Oct 25, 2018 at 05:12:59PM -0600, Jens Axboe wrote:
> On 10/23/18 12:07 PM, Jens Axboe wrote:
> > On 10/23/18 11:40 AM, Benjamin Block wrote:
> >>
> >> So I tried 4.19.0 with only the two patches from you:
> >> http://git.kernel.dk/cgit/linux-block/commit/?h=mq-conversions&id=2b2ffa16193e9a69a076595ed64429b8cc9b42aa
> >> http://git.kernel.dk/cgit/linux-block/commit/?h=mq-conversions&id=142dc9f36e3113b6a76d472978c33c8c2a2b702c
> >>
> >> This fixed the first warning from before, as you suggested, but it still
> >> crash like this:
> >>
> >> [ ] Unable to handle kernel pointer dereference in virtual kernel address
> >> space
> >> [ ] Failing address: TEID: 0483
> >> [ ] Fault in home space mode while using kernel ASCE.
> >> [ ] AS:025f0007 R3:dffb8007 S:dffbf000
> >> P:013d
> >> [ ] Oops: 0004 ilc:3 [#1] PREEMPT SMP DEBUG_PAGEALLOC
> >> [ ] Modules linked in:
> >> [ ] CPU: 2 PID: 609 Comm: kworker/2:1H Kdump: loaded Tainted: GW
> >> 4.19.0-bb-next+ #1
> >> [ ] Hardware name: IBM 3906 M03 704 (LPAR)
> >> [ ] Workqueue: kblockd blk_mq_run_work_fn
> >> [ ] Krnl PSW : 0704e0018000 03ff806a6b40
> >> (zfcp_fc_exec_bsg_job+0x1c0/0x440 [zfcp])
> >> [ ]R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:2 PM:0 RI:0 EA:3
> >> [ ] Krnl GPRS: 83e0f3c0
> >> 0300
> >> [ ]0300 03ff806a6b3a a86b5948
> >> a86b5988
> >> [ ]83e0f3f0 a86b5938
> >> 984aee80
> >> [ ]a86b5800 03ff806ba950 03ff806a6b3a
> >> 98a5ed88
> >> [ ] Krnl Code: 03ff806a6b30: b9040029lgr %r2,%r9
> >>03ff806a6b34: c0e58b7ebrasl
> >> %r14,3ff80698230
> >> #03ff806a6b3a: e310f0a4lg %r1,160(%r15)
> >> >03ff806a6b40: e3109024stg %r1,0(%r9)
> >>03ff806a6b46: 4120a040la %r2,64(%r10)
> >>03ff806a6b4a: c0e58ae7brasl
> >> %r14,3ff80698118
> >>03ff806a6b50: e310a044lg %r1,64(%r10)
> >>03ff806a6b56: e310f0a00024stg %r1,160(%r15)
> >> [ ] Call Trace:
> >> [ ] ([<03ff806a6aa2>] zfcp_fc_exec_bsg_job+0x122/0x440 [zfcp])
> >> [ ] [<03ff8061e928>] fc_bsg_dispatch+0x310/0x398 [scsi_transport_fc]
> >> [ ] [<00d2995a>] bsg_queue_rq+0x15a/0x198
> >> [ ] [<00d03566>] blk_mq_dispatch_rq_list+0x966/0xf90
> >> [ ] [<00d0e37a>] blk_mq_sched_dispatch_requests+0x3fa/0x410
> >> [ ] [<00cfc230>] __blk_mq_run_hw_queue+0x218/0x248
> >> [ ] [<001cb124>] process_one_work+0x8c4/0xe90
> >> [ ] [<001cbe58>] worker_thread+0x768/0xbb0
> >> [ ] [<001dc67a>] kthread+0x22a/0x248
> >> [ ] [<0137b812>] kernel_thread_starter+0x6/0xc
> >> [ ] [<0137b80c>] kernel_thread_starter+0x0/0xc
> >> [ ] INFO: lockdep is turned off.
> >> [ ] Last Breaking-Event-Address:
> >> [ ] [<005d9b08>] __asan_store8+0x98/0xa0
> >> [ ]
> >> [ ] Kernel panic - not syncing: Fatal exception: panic_on_oops
> >>
> >> zfcp_fc_exec_bsg_job+0x1c0 is here:
> >>
> >> int zfcp_fc_exec_bsg_job(struct bsg_job *job)
> >> {
> >> struct Scsi_Host *shost;
> >> struct zfcp_adapter *adapter;
> >> struct zfcp_fsf_ct_els *ct_els = job->dd_data;
> >> struct fc_bsg_request *bsg_request = job->request;
> >> struct fc_rport *rport = fc_bsg_to_rport(job);
> >>
> >> shost = rport ? rport_to_shost(rport) : fc_bsg_to_shost(job);
> >> adapter = (struct zfcp_adapter *)shost->hostdata[0];
> >>
> >> if (!(atomic_read(&adapter->status) & ZFCP_STATUS_COMMON_OPEN))
> >> return -EINVAL;
> >>
> >> ==> ct_els->req = job->request_payload.sg_list;
> >> ct_els->resp = job->reply_payload.sg_list;
> >> ct_els->handler_data = job;
> >>
> >> switch (bsg_request->msgcode) {
> >> case FC_BSG_RPT_ELS:
> >> case FC_BSG_HST_ELS_NOLOGIN:
> >> return zfcp_fc_exec_els_job(job, adapter);
> >> case FC_BSG_RPT_CT:
> >> case FC_BSG_HST_CT:
> >> return zfcp_fc_exec_ct_job(job, adapter);
> >> default:
> >> return -EINVAL;
> >> }
> >> }
> >>
> >> I took a dump to find out how struct bsg_job looks like when it crashes
> >> (register clobbering isn't as bad here and I verified that job->dev is
> >> valid).
> >>
> >> crash> print *((struct bsg_job *)0xa86b5938)
> >> $5 = {
> >> dev = 0x9af10358,
> >> kref = {
> >> refcount = {
> >> refs = {
> >> counter = 0x1
> >> }
> >>
Re: [PATCH v3 8/8] sg: user controls for q_at_head, read_value
Hi linux-scsi-owner,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on mkp-scsi/for-next]
[also build test ERROR on v4.19 next-20181019]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/linux-scsi-owner-vger-kernel-org/sg-types-and-naming-cleanup/20181026-220008
base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: i386-randconfig-x014-201842 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
In file included from include/linux/kernel.h:14:0,
from include/linux/list.h:9,
from include/linux/module.h:9,
from drivers/scsi/sg.c:22:
drivers/scsi/sg.c: In function 'init_sg':
>> drivers/scsi/sg.c:2277:3: error: 'sg_version_date' undeclared (first use in
>> this function); did you mean 'sg_version_num'?
sg_version_date);
^
include/linux/printk.h:315:34: note: in definition of macro 'pr_info'
printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
^~~
drivers/scsi/sg.c:2277:3: note: each undeclared identifier is reported only
once for each function it appears in
sg_version_date);
^
include/linux/printk.h:315:34: note: in definition of macro 'pr_info'
printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
^~~
drivers/scsi/sg.c: At top level:
drivers/scsi/sg.c:246:20: warning: 'sg_rq_state_str' used but never defined
static const char *sg_rq_state_str(enum sg_rq_state rq_state, bool
long_str);
^~~
vim +2277 drivers/scsi/sg.c
2256
2257 static int __init
2258 init_sg(void)
2259 {
2260 int rc;
2261
2262 if (scatter_elem_sz < PAGE_SIZE) {
2263 scatter_elem_sz = PAGE_SIZE;
2264 scatter_elem_sz_prev = scatter_elem_sz;
2265 }
2266 if (def_reserved_size >= 0)
2267 sg_big_buff = def_reserved_size;
2268 else
2269 def_reserved_size = sg_big_buff;
2270
2271 rc = register_chrdev_region(MKDEV(SCSI_GENERIC_MAJOR, 0),
2272 SG_MAX_DEVS, "sg");
2273 if (rc)
2274 return rc;
2275 pr_info("Registered %s[char major=0x%x], version: %s, date:
%s\n",
2276 "sg device ", SCSI_GENERIC_MAJOR, SG_VERSION_STR,
> 2277 sg_version_date);
2278 sg_sysfs_class = class_create(THIS_MODULE, "scsi_generic");
2279 if ( IS_ERR(sg_sysfs_class) ) {
2280 rc = PTR_ERR(sg_sysfs_class);
2281 goto err_out;
2282 }
2283 sg_sysfs_valid = 1;
2284 rc = scsi_register_interface(&sg_interface);
2285 if (0 == rc) {
2286 #ifdef CONFIG_SCSI_PROC_FS
2287 sg_proc_init();
2288 #endif /* CONFIG_SCSI_PROC_FS */
2289 return 0;
2290 }
2291 class_destroy(sg_sysfs_class);
2292 err_out:
2293 unregister_chrdev_region(MKDEV(SCSI_GENERIC_MAJOR, 0),
SG_MAX_DEVS);
2294 return rc;
2295 }
2296
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
Re: [PATCH 0/3] sd: Rely on the driver core for asynchronous probing
On Fri, 2018-10-26 at 15:58 +0200, Hannes Reinecke wrote: > On 10/2/18 11:52 PM, Bart Van Assche wrote: > > Hello Martin, > > > > During the 2018 edition of LSF/MM there was a session about increasing SCSI > > disk probing concurrency. This patch series implements what has been > > proposed > > during that session, namely: > > - Make sure that the driver core is aware of asynchronous SCSI LUN probing. > > - Avoid unnecessary serialization between sd_probe() and sd_remove() because > >this could lead to a deadlock. > > > > Please consider this patch series for kernel v4.20. > > > > Thanks, > > > > Bart. > > > > Bart Van Assche (3): > >sd: Rely on the driver core for asynchronous probing > >sd: Inline sd_probe_part2() > >__device_release_driver(): Do not wait for asynchronous probing > > > > drivers/base/dd.c| 3 -- > > drivers/scsi/scsi.c | 14 - > > drivers/scsi/scsi_pm.c | 22 +--- > > drivers/scsi/scsi_priv.h | 3 -- > > drivers/scsi/sd.c| 110 --- > > 5 files changed, 46 insertions(+), 106 deletions(-) > > > > Initially it looks ok-ish. > But I've has so many issues with asynchronous probing that I'd be really > careful here. > However, inlining sd_probe_async() should amount for some contention to > be removed; but I'd rather give it some more testing here. > I've found that testing on large configurations on iSER or SRP tend to > excite the issues. > Can you check against those, too? > I'll see to give it a spin on my test bed. Hi Hannes, As you may have noticed the build bot was not very happy with the patches in v2 of this series for the device driver core. Additional testing is welcome but please start from the patches in the following branch for any tests: https://github.com/bvanassche/linux/tree/for-next. On that branch my patches for the device driver core have been replaced with patches from Alexander Duyck. Thanks, Bart.
Re: [PATCH 05/28] IB/srp: remove old request_fn_active check
On Fri, 2018-10-26 at 08:32 -0600, Jens Axboe wrote:
> On 10/26/18 1:08 AM, Hannes Reinecke wrote:
> > On 10/25/18 11:10 PM, Jens Axboe wrote:
> > > This check is only viable for non scsi-mq. Since that is going away,
> > > kill this legacy check.
> > >
> > > Cc: Bart Van Assche
> > > Cc: Parav Pandit
> > > Cc: linux-scsi@vger.kernel.org
> > > Signed-off-by: Jens Axboe
> > > ---
> > > drivers/infiniband/ulp/srp/ib_srp.c | 7 ---
> > > 1 file changed, 7 deletions(-)
> > >
> > > diff --git a/drivers/infiniband/ulp/srp/ib_srp.c
> > > b/drivers/infiniband/ulp/srp/ib_srp.c
> > > index 0b34e909505f..5a79444c2f3c 100644
> > > --- a/drivers/infiniband/ulp/srp/ib_srp.c
> > > +++ b/drivers/infiniband/ulp/srp/ib_srp.c
> > > @@ -1334,13 +1334,6 @@ static void srp_terminate_io(struct srp_rport
> > > *rport)
> > > struct scsi_device *sdev;
> > > int i, j;
> > >
> > > - /*
> > > - * Invoking srp_terminate_io() while srp_queuecommand() is running
> > > - * is not safe. Hence the warning statement below.
> > > - */
> > > - shost_for_each_device(sdev, shost)
> > > - WARN_ON_ONCE(sdev->request_queue->request_fn_active);
> > > -
> > > for (i = 0; i < target->ch_count; i++) {
> > > ch = &target->ch[i];
> > >
> > >
> >
> > I _think_ I've already killed that one; please do check the latest tree
> > from Doug Ledford.
>
> I don't know which tree is Doug's, but as long as it's queued up
> for mainline, that's all I care about. I'm still rebasing this
> branch, so it'll just get dropped when it shows up in mainline.
Doug and Jason share a git repository. I think this is the git repository they
ask Linus to pull from:
git://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git.
Although Doug had reported to have queued the above patch I haven't found that
patch
in the RDMA git repository.
Bart.
Re: scsi: myrs: warning fix, was: [GIT PULL] first round of SCSI updates for the 4.19+ merge window
On 10/26/18, Hannes Reinecke wrote: > On 10/26/18 10:16 AM, Arnd Bergmann wrote: >> On Wed, Oct 24, 2018 at 1:00 PM James Bottomley >> wrote: >>> >>> James Bottomley (1): >>>scsi: myrs: fix build failure on 32 bit >> >> Hi James and Hannes, >> >> Since James mentioned 32-bit compiles during the kernel summit, >> I'd like to confirm that I hit this on my randconfig builder now, >> with some latency since the last linux-next tree I tested before >> flying to Edinburgh did not have the bug, and the latest >> linux-next tree that is available now (dated last Friday) does, and >> I see your tree is fixed. During normal times, I should catch these >> within a short time of the patch getting into scsi-next. >> >> However, while looking at this bug, I found two more issues related >> to the specific computation: >> >> percent_complete = ldev_info->rbld_lba * 100 / ldev_info->cfg_devsize; >> >> I see that both rbld_lba and cfg_devsize are reported by the >> device, but only the former is 64 bit but the latter is 32 bit and >> also intended to be the larger of the two. I suspect this is a >> bug, and the same is also present in the old DAC960.c. >> cfg_devsize is followed by four reserved bytes in the header, >> so I suppose it was meant to be 64-bit? >> If you divide two 64-bit numbers, you also have to use div_u64_64() >> instead of do_div(). >> >> On top of that, I see we get those values from the device but >> never do any endianess conversion on them. It seems likely >> that they are all little-endian and require a le32_to_cpu() >> conversion to also work on big-endian kernel builds. Alternatively >> we could make the Kconfig symbol as >> 'depends on !CPU_BIG_ENDIAN || COMPILE_TEST'. >> > It _really_ is questionable if these device ever work on big-endian > machines, as they rely on the BIOS to start up the RAID engine; I've had > a hard enough time getting them to work on normal machines :-) Shall we add the Kconfig dependency then? I can send a patch for that. > Plus the firmware refused to handle any drive larger than 4GB (!), so > it's really a purely theoretical issue whether 'cfgsize' was meant to be > 64 bit ... I see. It was likely meant as a possible extension for future firmware version then, which may or may not have been developed or released. I suppose the do_div() could be replaced with 32-bit arithmetic, but it's not in a fast path and James' version should be correct as well, so I won't send another patch for that one. Arnd
Re: [PATCH 05/28] IB/srp: remove old request_fn_active check
On 10/26/18 1:08 AM, Hannes Reinecke wrote:
> On 10/25/18 11:10 PM, Jens Axboe wrote:
>> This check is only viable for non scsi-mq. Since that is going away,
>> kill this legacy check.
>>
>> Cc: Bart Van Assche
>> Cc: Parav Pandit
>> Cc: linux-scsi@vger.kernel.org
>> Signed-off-by: Jens Axboe
>> ---
>> drivers/infiniband/ulp/srp/ib_srp.c | 7 ---
>> 1 file changed, 7 deletions(-)
>>
>> diff --git a/drivers/infiniband/ulp/srp/ib_srp.c
>> b/drivers/infiniband/ulp/srp/ib_srp.c
>> index 0b34e909505f..5a79444c2f3c 100644
>> --- a/drivers/infiniband/ulp/srp/ib_srp.c
>> +++ b/drivers/infiniband/ulp/srp/ib_srp.c
>> @@ -1334,13 +1334,6 @@ static void srp_terminate_io(struct srp_rport *rport)
>> struct scsi_device *sdev;
>> int i, j;
>>
>> -/*
>> - * Invoking srp_terminate_io() while srp_queuecommand() is running
>> - * is not safe. Hence the warning statement below.
>> - */
>> -shost_for_each_device(sdev, shost)
>> -WARN_ON_ONCE(sdev->request_queue->request_fn_active);
>> -
>> for (i = 0; i < target->ch_count; i++) {
>> ch = &target->ch[i];
>>
>>
> I _think_ I've already killed that one; please do check the latest tree
> from Doug Ledford.
I don't know which tree is Doug's, but as long as it's queued up
for mainline, that's all I care about. I'm still rebasing this
branch, so it'll just get dropped when it shows up in mainline.
--
Jens Axboe
Re: scsi: myrs: warning fix, was: [GIT PULL] first round of SCSI updates for the 4.19+ merge window
On 10/26/18 10:16 AM, Arnd Bergmann wrote: On Wed, Oct 24, 2018 at 1:00 PM James Bottomley wrote: James Bottomley (1): scsi: myrs: fix build failure on 32 bit Hi James and Hannes, Since James mentioned 32-bit compiles during the kernel summit, I'd like to confirm that I hit this on my randconfig builder now, with some latency since the last linux-next tree I tested before flying to Edinburgh did not have the bug, and the latest linux-next tree that is available now (dated last Friday) does, and I see your tree is fixed. During normal times, I should catch these within a short time of the patch getting into scsi-next. However, while looking at this bug, I found two more issues related to the specific computation: percent_complete = ldev_info->rbld_lba * 100 / ldev_info->cfg_devsize; I see that both rbld_lba and cfg_devsize are reported by the device, but only the former is 64 bit but the latter is 32 bit and also intended to be the larger of the two. I suspect this is a bug, and the same is also present in the old DAC960.c. cfg_devsize is followed by four reserved bytes in the header, so I suppose it was meant to be 64-bit? If you divide two 64-bit numbers, you also have to use div_u64_64() instead of do_div(). On top of that, I see we get those values from the device but never do any endianess conversion on them. It seems likely that they are all little-endian and require a le32_to_cpu() conversion to also work on big-endian kernel builds. Alternatively we could make the Kconfig symbol as 'depends on !CPU_BIG_ENDIAN || COMPILE_TEST'. It _really_ is questionable if these device ever work on big-endian machines, as they rely on the BIOS to start up the RAID engine; I've had a hard enough time getting them to work on normal machines :-) Plus the firmware refused to handle any drive larger than 4GB (!), so it's really a purely theoretical issue whether 'cfgsize' was meant to be 64 bit ... Cheers, Hannes -- Dr. Hannes ReineckezSeries & Storage h...@suse.com +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)
[Bug 201199] hpsa blocking boot on HP smart Array P410
https://bugzilla.kernel.org/show_bug.cgi?id=201199 --- Comment #8 from Seb Lu (se...@seblu.net) --- Hello, Any idea? Any test I can do to find what's wrong? -- You are receiving this mail because: You are the assignee for the bug.
Re: [PATCH 0/3] sd: Rely on the driver core for asynchronous probing
On 10/2/18 11:52 PM, Bart Van Assche wrote: Hello Martin, During the 2018 edition of LSF/MM there was a session about increasing SCSI disk probing concurrency. This patch series implements what has been proposed during that session, namely: - Make sure that the driver core is aware of asynchronous SCSI LUN probing. - Avoid unnecessary serialization between sd_probe() and sd_remove() because this could lead to a deadlock. Please consider this patch series for kernel v4.20. Thanks, Bart. Bart Van Assche (3): sd: Rely on the driver core for asynchronous probing sd: Inline sd_probe_part2() __device_release_driver(): Do not wait for asynchronous probing drivers/base/dd.c| 3 -- drivers/scsi/scsi.c | 14 - drivers/scsi/scsi_pm.c | 22 +--- drivers/scsi/scsi_priv.h | 3 -- drivers/scsi/sd.c| 110 --- 5 files changed, 46 insertions(+), 106 deletions(-) Initially it looks ok-ish. But I've has so many issues with asynchronous probing that I'd be really careful here. However, inlining sd_probe_async() should amount for some contention to be removed; but I'd rather give it some more testing here. I've found that testing on large configurations on iSER or SRP tend to excite the issues. Can you check against those, too? I'll see to give it a spin on my test bed. Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)
[PATCH v3 6/8] sg: complete locking changes on ioctl+debug
Complete the locking and structure changes of ioctl and debug
('cat /proc/scsi/sg/debug') handling.
Signed-off-by: Douglas Gilbert
---
This was the code that was "#if 0'-ed out 2 patches ago. It
also shuts checkpatch.pl up as it doesn't like that technique
but offers no viable substitute.
drivers/scsi/sg.c | 217 +-
1 file changed, 136 insertions(+), 81 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 5fbdb0f40739..8b4a65340971 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -239,6 +239,7 @@ static struct sg_request *sg_add_request(struct sg_fd *sfp,
int dxfr_len,
static void sg_remove_request(struct sg_fd *sfp, struct sg_request *srp);
static struct sg_device *sg_get_dev(int min_dev);
static void sg_device_destroy(struct kref *kref);
+static const char *sg_rq_state_str(u8 rq_state, bool long_str);
#define SZ_SG_HEADER sizeof(struct sg_header) /* v1 and v2 header */
#define SZ_SG_IO_HDR sizeof(struct sg_io_hdr) /* v3 header */
@@ -359,7 +360,7 @@ sg_open(struct inode *inode, struct file *filp)
nonseekable_open(inode, filp);
if ((flags & O_EXCL) && (O_RDONLY == (flags & O_ACCMODE)))
- return -EPERM; /* Can't lock it with read only access */
+ return -EPERM;/* not permitted, need write access for O_EXCL */
sdp = sg_get_dev(min_dev);
if (IS_ERR(sdp))
return PTR_ERR(sdp);
@@ -931,11 +932,6 @@ sg_ktime_sub_trunc(ktime_t now_ts, ktime_t ts0)
return 0;
}
-/*
- * Annotation under function arguments (i.e. '__must_hold...') states that
- * this function expects that lock to be held, a read lock is sufficient in
- * this case.
- */
static void
sg_fill_request_element(struct sg_fd *sfp, struct sg_request *srp,
struct sg_req_info *rip)
@@ -982,7 +978,6 @@ sg_fill_request_element(struct sg_fd *sfp, struct
sg_request *srp,
* Populate up to max_num struct sg_req_info objects, first from the active
* list then, if there is still room, from the free list. All elements in
* the free list should have SG_RQ_INACTIVE status.
- * See sg_fill_request_element() for note about __must_hold annotation.
*/
static void
sg_fill_request_table(struct sg_fd *sfp, struct sg_req_info *rinfo,
@@ -1031,7 +1026,6 @@ srp_state_or_detaching(struct sg_device *sdp, struct
sg_request *srp)
return ret;
}
-#if 0 /* temporary to shorten big patch */
static long
sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
{
@@ -1063,24 +1057,37 @@ sg_ioctl(struct file *filp, unsigned int cmd_in,
unsigned long arg)
return -ENXIO;
if (!access_ok(VERIFY_WRITE, p, SZ_SG_IO_HDR))
return -EFAULT;
- result = sg_new_write(sfp, filp, p, SZ_SG_IO_HDR,
-1, read_only, 1, &srp);
+ result = sg_v3_write(sfp, filp, p, SZ_SG_IO_HDR, read_only,
+true, &srp);
if (result < 0)
return result;
result = wait_event_interruptible(sfp->read_wait,
- (srp_done(sfp, srp) || atomic_read(&sdp->detaching)));
- if (atomic_read(&sdp->detaching))
+ srp_state_or_detaching(sdp, srp));
+
+ spin_lock_irqsave(&srp->rq_entry_lck, iflags);
+ if (unlikely(result)) { /* -ERESTARTSYS because signal hit */
+ srp->orphan = true;
+ srp->rq_state = SG_RQ_INFLIGHT;
+ spin_unlock_irqrestore(&srp->rq_entry_lck, iflags);
+ SG_LOG(1, sdp, "%s: wait_event_interruptible-->%d\n",
+ __func__, result);
+ return result;
+ }
+ if (unlikely(atomic_read(&sdp->detaching))) {
+ srp->rq_state = SG_RQ_INACTIVE;
+ spin_unlock_irqrestore(&srp->rq_entry_lck, iflags);
return -ENODEV;
- write_lock_irq(&sfp->rq_list_lock);
- if (srp->done) {
- srp->done = 2;
- write_unlock_irq(&sfp->rq_list_lock);
+ } else if (likely(srp->rq_state == SG_RQ_AWAIT_READ)) {
+ srp->rq_state = SG_RQ_DONE_READ;
+ spin_unlock_irqrestore(&srp->rq_entry_lck, iflags);
result = sg_new_read(sfp, p, SZ_SG_IO_HDR, srp);
return (result < 0) ? result : 0;
}
- srp->orphan = true;
- write_unlock_irq(&sfp->rq_list_lock);
- return result; /* -ERESTARTSYS because signal hit process */
+ cp = sg_rq_state_str(srp->rq_state, true);
+ SG_LOG(1, sdp, "%s: unexpected srp=0x%p state: %s\n", __func__,
+
[PATCH v3 3/8] sg: split header, expand and correct descriptions
Split scsi/sg.h into a smaller scsi/sg.h which includes a new header:
uapi/scsi/sg.h . Overall expand the twin header files with new
functionality in this patchset and functionality to be added in the
next patchset to implement SG_IOSUBMIT and friends. Adjust format to
modern kernel conventions. Correct and expand some comments.
Signed-off-by: Douglas Gilbert
---
The new header file: uapi/scsi/sg.h is expected to be in the
kernel's public interface. This takes time (i.e. months or years)
to find its way into glibc and Linux distributions. So the sooner
it is presented and accepted the better.
>From the C perspective, nothing is removed or changed (or shouldn't
be), only additions. This means that the original typedefs stay (but
they are not used in the driver source file: sg.c) since user space
programs may be using them.
include/scsi/sg.h | 252 ++---
include/uapi/scsi/sg.h | 419 +
2 files changed, 430 insertions(+), 241 deletions(-)
create mode 100644 include/uapi/scsi/sg.h
diff --git a/include/scsi/sg.h b/include/scsi/sg.h
index f91bcca604e4..596f68746f66 100644
--- a/include/scsi/sg.h
+++ b/include/scsi/sg.h
@@ -7,23 +7,23 @@
/*
* History:
* Started: Aug 9 by Lawrence Foard (entr...@world.std.com), to allow user
- * process control of SCSI devices.
+ * process control of SCSI devices.
* Development Sponsored by Killy Corp. NY NY
*
* Original driver (sg.h):
- * Copyright (C) 1992 Lawrence Foard
+ * Copyright (C) 1992 Lawrence Foard
* Version 2 and 3 extensions to driver:
- * Copyright (C) 1998 - 2014 Douglas Gilbert
+ * Copyright (C) 1998 - 2018 Douglas Gilbert
*
- * Version: 3.5.36 (20140603)
- * This version is for 2.6 and 3 series kernels.
+ * Version: 3.9.01 (20181016)
+ * This version is for 2.6, 3 and 4 series kernels.
*
* Documentation
* =
* A web site for the SG device driver can be found at:
- * http://sg.danny.cz/sg [alternatively check the MAINTAINERS file]
+ * http://sg.danny.cz/sg [alternatively check the MAINTAINERS file]
* The documentation for the sg version 3 driver can be found at:
- * http://sg.danny.cz/sg/p/sg_v3_ho.html
+ * http://sg.danny.cz/sg/p/sg_v3_ho.html
* Also see: /Documentation/scsi/scsi-generic.txt
*
* For utility and test programs see: http://sg.danny.cz/sg/sg3_utils.html
@@ -33,242 +33,12 @@
extern int sg_big_buff; /* for sysctl */
#endif
+#include
-typedef struct sg_iovec /* same structure as used by readv() Linux system */
-{ /* call. It defines one scatter-gather element. */
-void __user *iov_base; /* Starting address */
-size_t iov_len; /* Length in bytes */
-} sg_iovec_t;
-
-
-typedef struct sg_io_hdr
-{
-int interface_id; /* [i] 'S' for SCSI generic (required) */
-int dxfer_direction;/* [i] data transfer direction */
-unsigned char cmd_len; /* [i] SCSI command length */
-unsigned char mx_sb_len;/* [i] max length to write to sbp */
-unsigned short iovec_count; /* [i] 0 implies no scatter gather */
-unsigned int dxfer_len; /* [i] byte count of data transfer */
-void __user *dxferp; /* [i], [*io] points to data transfer memory
- or scatter gather list */
-unsigned char __user *cmdp; /* [i], [*i] points to command to perform */
-void __user *sbp; /* [i], [*o] points to sense_buffer memory */
-unsigned int timeout; /* [i] MAX_UINT->no timeout (unit: millisec) */
-unsigned int flags; /* [i] 0 -> default, see SG_FLAG... */
-int pack_id;/* [i->o] unused internally (normally) */
-void __user * usr_ptr; /* [i->o] unused internally */
-unsigned char status; /* [o] scsi status */
-unsigned char masked_status;/* [o] shifted, masked scsi status */
-unsigned char msg_status; /* [o] messaging level data (optional) */
-unsigned char sb_len_wr;/* [o] byte count actually written to sbp */
-unsigned short host_status; /* [o] errors from host adapter */
-unsigned short driver_status;/* [o] errors from software driver */
-int resid; /* [o] dxfer_len - actual_transferred */
-unsigned int duration; /* [o] time taken by cmd (unit: millisec) */
-unsigned int info; /* [o] auxiliary information */
-} sg_io_hdr_t; /* 64 bytes long (on i386) */
-
-#define SG_INTERFACE_ID_ORIG 'S'
-
-/* Use negative values to flag difference from original sg_header structure */
-#define SG_DXFER_NONE (-1) /* e.g. a SCSI Test Unit Ready command */
-#define SG_DXFER_TO_DEV (-2)/* e.g. a SCSI WRITE command */
-#define SG_DXFER_FROM_DEV (-3) /* e.g. a SCSI READ command */
-#define SG_DXFER_TO_FROM_DEV (-4) /* treated like SG_DXFER_FROM_DEV with the
- additional property than during indirect
-
[PATCH v3 4/8] sg: expand request states
Introduce the new sg_rq_state enumerations for tracking the
lifetime of a sg_request. SG_RQ_DATA_THRESHOLD is a
default value that if the data length of a request exceeds
then, after that request is completed, the data buffer will
be freed up as the sg_request object is placed on the free
list. SG_TOT_FD_THRESHOLD is a default, per file descriptor
value that the sum of outstanding command data lengths is not
allowed to exceed.
Signed-off-by: Douglas Gilbert
---
The '#if 0's are temporary and removed in a later patch.
They allow the following large and complex patch (5 of 8)
to be a bit shorter and still compile.
drivers/scsi/sg.c | 29 +
1 file changed, 29 insertions(+)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 94e13a1d21a5..a76395f16fb1 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -75,6 +75,24 @@ static int sg_proc_init(void);
*/
#define SG_MAX_CDB_SIZE 252
+/* Following enum contains the states of sg_request::rq_state */
+enum sg_rq_state {
+ SG_RQ_INACTIVE = 0, /* request not in use (e.g. on fl) */
+ SG_RQ_INFLIGHT, /* SCSI request issued, no response yet */
+ SG_RQ_AWAIT_READ, /* response received, awaiting read */
+ SG_RQ_DONE_READ,/* read is ongoing or done */
+ SG_RQ_BUSY, /* example: reserve request changing size */
+};
+
+/* free up requests larger than this dlen size after use */
+#define SG_RQ_DATA_THRESHOLD (128 * 1024)
+
+/* If sum_of(dlen) of a fd exceeds this, write() will yield E2BIG */
+#define SG_TOT_FD_THRESHOLD (16 * 1024 * 1024)
+
+#define SG_TIME_UNIT_MS 0 /* milliseconds */
+#define SG_TIME_UNIT_NS 1 /* nanoseconds */
+#define SG_DEF_TIME_UNIT SG_TIME_UNIT_MS
#define SG_DEFAULT_TIMEOUT mult_frac(SG_DEFAULT_TIMEOUT_USER, HZ, USER_HZ)
int sg_big_buff = SG_DEF_RESERVED_SIZE;
@@ -950,6 +968,7 @@ sg_fill_request_table(struct sg_fd *sfp, struct sg_req_info
*rinfo)
}
}
+#if 0 /* temporary to shorten big patch */
static long
sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
{
@@ -1227,6 +1246,7 @@ sg_compat_ioctl(struct file *filp, unsigned int cmd_in,
unsigned long arg)
return -ENOIOCTLCMD;
}
#endif
+#endif /* temporary to shorten big patch */
static __poll_t
sg_poll(struct file *filp, poll_table * wait)
@@ -1496,10 +1516,12 @@ static const struct file_operations sg_fops = {
.read = sg_read,
.write = sg_write,
.poll = sg_poll,
+#if 0 /* temporary to shorten big patch */
.unlocked_ioctl = sg_ioctl,
#ifdef CONFIG_COMPAT
.compat_ioctl = sg_compat_ioctl,
#endif
+#endif /* temporary to shorten big patch */
.open = sg_open,
.mmap = sg_mmap,
.release = sg_release,
@@ -2422,12 +2444,16 @@ static const struct seq_operations devstrs_seq_ops = {
.show = sg_proc_seq_show_devstrs,
};
+#if 0 /* temporary to shorten big patch */
static int sg_proc_seq_show_debug(struct seq_file *s, void *v);
+#endif /* temporary to shorten big patch */
static const struct seq_operations debug_seq_ops = {
.start = dev_seq_start,
.next = dev_seq_next,
.stop = dev_seq_stop,
+#if 0 /* temporary to shorten big patch */
.show = sg_proc_seq_show_debug,
+#endif /* temporary to shorten big patch */
};
static int
@@ -2601,6 +2627,8 @@ sg_proc_seq_show_devstrs(struct seq_file *s, void *v)
return 0;
}
+#if 0 /* temporary to shorten big patch */
+
/* must be called while holding sg_index_lock */
static void
sg_proc_debug_helper(struct seq_file *s, struct sg_device *sdp)
@@ -2704,6 +2732,7 @@ sg_proc_seq_show_debug(struct seq_file *s, void *v)
read_unlock_irqrestore(&sg_index_lock, iflags);
return 0;
}
+#endif /* temporary to shorten big patch */
#endif /* CONFIG_SCSI_PROC_FS */
--
2.17.1
[PATCH v3 7/8] sg: rework ioctl handling
Rework ioctl handling, report clearly to the log which ioctl has
been invoked. Add a new "IOWR" ioctl: SG_SET_GET_EXTENDED which
permits several integer and boolean values to be "_SET_" (i.e.
passed into the driver, potentially changing its actions) and/or
read from the driver (the "_GET_" part) in a single operation.
Signed-off-by: Douglas Gilbert
---
One feature of the new SG_SET_GET_EXTENDED ioctl is ability to
fetch the sg device minor number (e.g. the "3" in /dev/sg3)
associated with the current file descriptor. A boolean addition
is the ability to change command timekeeping on the current file
descriptor from units of milliseconds (the default) to
nanoseconds.
drivers/scsi/sg.c | 535 +++---
1 file changed, 409 insertions(+), 126 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 8b4a65340971..3e89bbd508de 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -90,8 +90,8 @@ enum sg_rq_state {
/* If sum_of(dlen) of a fd exceeds this, write() will yield E2BIG */
#define SG_TOT_FD_THRESHOLD (16 * 1024 * 1024)
-#define SG_TIME_UNIT_MS 0 /* milliseconds */
-#define SG_TIME_UNIT_NS 1 /* nanoseconds */
+#define SG_TIME_UNIT_MS 0 /* milliseconds */
+#define SG_TIME_UNIT_NS 1 /* nanoseconds */
#define SG_DEF_TIME_UNIT SG_TIME_UNIT_MS
#define SG_DEFAULT_TIMEOUT mult_frac(SG_DEFAULT_TIMEOUT_USER, HZ, USER_HZ)
@@ -186,7 +186,6 @@ struct sg_fd { /* holds the state of a file
descriptor */
bool cmd_q; /* true -> allow command queuing, false -> don't */
bool keep_orphan;/* false -> drop (def), true -> keep for read() */
bool mmap_called; /* false -> mmap() never called on this fd */
- bool sse_seen; /* SG_SET_EXTENDED ioctl seen */
bool time_in_ns;/* report times in nanoseconds */
u8 next_cmd_len;/* 0: automatic, >0: use on next write() */
struct sg_request *reserve_srp; /* allocate on open(), starts on fl */
@@ -240,6 +239,8 @@ static void sg_remove_request(struct sg_fd *sfp, struct
sg_request *srp);
static struct sg_device *sg_get_dev(int min_dev);
static void sg_device_destroy(struct kref *kref);
static const char *sg_rq_state_str(u8 rq_state, bool long_str);
+static struct sg_request *sg_mk_srp(struct sg_fd *sfp, bool first,
+ rwlock_t *rwlp, unsigned long *iflagsp);
#define SZ_SG_HEADER sizeof(struct sg_header) /* v1 and v2 header */
#define SZ_SG_IO_HDR sizeof(struct sg_io_hdr) /* v3 header */
@@ -508,9 +509,9 @@ sg_read(struct file *filp, char __user *buf, size_t count,
loff_t * ppos)
return IS_ERR(sfp) ? PTR_ERR(sfp) : -ENXIO;
}
sdp = sfp->parentdp;
+ SG_LOG(3, sdp, "%s: read() count=%d\n", __func__, (int)count);
if (IS_ERR_OR_NULL(sdp))
return IS_ERR(sdp) ? PTR_ERR(sdp) : -ENXIO;
- SG_LOG(3, sdp, "%s: read() count=%d\n", __func__, (int)count);
if (!access_ok(VERIFY_WRITE, buf, count))
return -EFAULT;
@@ -815,11 +816,11 @@ sg_v3_write(struct sg_fd *sfp, struct file *file, const
char __user *buf,
return -ENOSYS;
if (hp->flags & SG_FLAG_MMAP_IO) {
if (!list_empty(&sfp->rq_list))
- return -EBUSY; /* already active requests on fd */
+ return -EBUSY; /* already active requests on fd */
if (hp->dxfer_len > sfp->reserve_srp->data.dlen)
- return -ENOMEM; /* MMAP_IO size must fit in reserve */
+ return -ENOMEM; /* MMAP_IO size must fit in reserve */
if (hp->flags & SG_FLAG_DIRECT_IO)
- return -EINVAL; /* not both MMAP_IO and DIRECT_IO */
+ return -EINVAL; /* not both MMAP_IO and DIRECT_IO */
}
sfp->cmd_q = true; /* when sg_io_hdr seen, set command queuing on */
ul_timeout = msecs_to_jiffies(hp->timeout);
@@ -857,7 +858,7 @@ sg_common_write(struct sg_fd *sfp, const struct sg_io_hdr
*hi_p,
srp = sg_add_request(sfp, hi_p->dxfer_len, false);
if (IS_ERR(srp))
return srp;
- srp->header = *hi_p;/* structure assignment, could memcpy */
+ srp->header = *hi_p;/* structure assignment, could memcpy */
hp = &srp->header;
srp->data.cmd_opcode = cmnd[0]; /* hold opcode of command */
hp->status = 0;
@@ -1026,69 +1027,300 @@ srp_state_or_detaching(struct sg_device *sdp, struct
sg_request *srp)
return ret;
}
+/* For handling ioctl(SG_IO). Returns 0 on success else a negated errno */
+static int
+sg_sg_io(struct file *filp, struct sg_device *sdp, struct sg_fd *sfp,
+void __user *p)
+{
+ bool read_only = (O_RDWR != (filp->f_flags & O_ACCMODE));
+ int result;
+ unsigned long iflags;
+ struct sg_request *srp;
+ const char *cp;
+
+ SG_LOG(3, sdp, "
[PATCH v3 8/8] sg: user controls for q_at_head, read_value
Add a SG_SET_GET_EXTENDED ioctl control for whether commands
will be queued_at_head or queued_at_tail by the block layer
(together with the scsi mid-level). It has file scope.
Also add a read_value integer the can be used by write a
value from the SG_SEIRV_* group then the corresponding value
will be returned.
Signed-off-by: Douglas Gilbert
---
The user can still override the new file scope setting on a
a per command basis with the SG_FLAG_Q_AT_HEAD and
SG_FLAG_Q_AT_TAIL in the sg v3 and v4 structures.
An example of read_value usage is to write the value
SG_SEIRV_FL_RQS to the read_value field. Then after the
SG_SET_GET_EXTENDED ioctl is run, the number of (inactive)
requests currently on this file descriptor's request free
list is placed in the read_value field.
Added in v3 is SG_SEIRV_DEV_FL_RQS which is an expansion of
SG_SEIRV_FL_RQS. SG_SEIRV_DEV_FL_RQS counts free list entries
on all sg file descriptors currently open on the device that
the file descriptor (given to ioctl()) is associated with.
drivers/scsi/sg.c | 160 +++--
include/scsi/sg.h | 32 ++---
include/uapi/scsi/sg.h | 49 ++---
3 files changed, 150 insertions(+), 91 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 3e89bbd508de..4d6966d40949 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -59,7 +59,7 @@ static int sg_version_num = 30901;/* 2 digits for each
component */
#ifdef CONFIG_SCSI_PROC_FS
#include
-static char *sg_version_date = "20181019";
+static char *sg_version_date = "20181024";
static int sg_proc_init(void);
#endif
@@ -95,6 +95,10 @@ enum sg_rq_state {
#define SG_DEF_TIME_UNIT SG_TIME_UNIT_MS
#define SG_DEFAULT_TIMEOUT mult_frac(SG_DEFAULT_TIMEOUT_USER, HZ, USER_HZ)
+#define SG_FD_Q_AT_TAIL true
+#define SG_FD_Q_AT_HEAD false
+#define SG_DEFAULT_Q_AT SG_FD_Q_AT_HEAD/* for backward compatibility */
+
int sg_big_buff = SG_DEF_RESERVED_SIZE;
/* N.B. This variable is readable and writeable via
/proc/scsi/sg/def_reserved_size . Each time sg_open() is called a buffer
@@ -187,6 +191,7 @@ struct sg_fd { /* holds the state of a file
descriptor */
bool keep_orphan;/* false -> drop (def), true -> keep for read() */
bool mmap_called; /* false -> mmap() never called on this fd */
bool time_in_ns;/* report times in nanoseconds */
+ bool q_at_tail; /* queue at tail if true, head when false */
u8 next_cmd_len;/* 0: automatic, >0: use on next write() */
struct sg_request *reserve_srp; /* allocate on open(), starts on fl */
struct fasync_struct *async_qp; /* used by asynchronous notification */
@@ -238,7 +243,7 @@ static struct sg_request *sg_add_request(struct sg_fd *sfp,
int dxfr_len,
static void sg_remove_request(struct sg_fd *sfp, struct sg_request *srp);
static struct sg_device *sg_get_dev(int min_dev);
static void sg_device_destroy(struct kref *kref);
-static const char *sg_rq_state_str(u8 rq_state, bool long_str);
+static const char *sg_rq_state_str(enum sg_rq_state rq_state, bool long_str);
static struct sg_request *sg_mk_srp(struct sg_fd *sfp, bool first,
rwlock_t *rwlp, unsigned long *iflagsp);
@@ -855,7 +860,7 @@ sg_common_write(struct sg_fd *sfp, const struct sg_io_hdr
*hi_p,
if (h4p || !hi_p)
return ERR_PTR(-EOPNOTSUPP);
- srp = sg_add_request(sfp, hi_p->dxfer_len, false);
+ srp = sg_add_request(sfp, hi_p->dxfer_len, sync);
if (IS_ERR(srp))
return srp;
srp->header = *hi_p;/* structure assignment, could memcpy */
@@ -897,9 +902,13 @@ sg_common_write(struct sg_fd *sfp, const struct sg_io_hdr
*hi_p,
srp->start_ts = ktime_get_with_offset(TK_OFFS_BOOT);
else
hp->duration = jiffies_to_msecs(jiffies);
- /* at tail if v3 or later interface and tail flag set */
- at_head = !(hp->interface_id != '\0' &&
- (SG_FLAG_Q_AT_TAIL & hp->flags));
+
+ if (hp->interface_id == '\0') /* v1 and v2 interface */
+ at_head = true; /* backward compatibility */
+ else if (sfp->q_at_tail) /* cmd flags can override sfd setting */
+ at_head = (hp->flags & SG_FLAG_Q_AT_HEAD);
+ else/* this sfd is defaulting to head */
+ at_head = !(hp->flags & SG_FLAG_Q_AT_TAIL);
srp->rq->timeout = timeout;
kref_get(&sfp->f_ref); /* sg_rq_end_io() does kref_put(). */
@@ -1084,30 +1093,30 @@ sg_reserved_sz(struct sg_fd *sfp, struct
sg_extended_info *seip)
{
bool free_n_srp = false;
int result = 0;
- int val, mx_sect_bytes;
+ int new_sz, mx_sect_bytes;
unsigned long iflags;
- struct sg_request *srp; /* prior reserve request */
+ struct sg_request *o_srp; /* prior reserve request */
struct sg_request *
[PATCH v3 5/8] sg: add free list, rework locking
Remove fixed 16 sg_request object array and replace with an active
rq_list plus a request free list. Add finer grained spin lock to
sg_request and do a major rework on locking. sg_request objects now
are only de-allocated when the owning file descriptor is closed.
This simplifies locking issues.
Signed-off-by: Douglas Gilbert
---
This patch is big and complex. Towards the end the diff program
completely loses the plot. Better to use difftool on a two pane
window, or simply view the before sg.c and the after sg.c .
The requirement to keep the patch small enough to be reviewable
but at the same time both compilable and buildable (i.e. no
undefines when kernel is built) are in conflict, especially when
the changes touch almost all functions in a driver. IMO you
should be able to mark a patch (in the middle of a patchset)
as "non compilable".
drivers/scsi/sg.c | 1305 -
1 file changed, 805 insertions(+), 500 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index a76395f16fb1..5fbdb0f40739 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -141,46 +141,58 @@ struct sg_scatter_hold { /* holding area for scsi
scatter gather info */
struct sg_device; /* forward declarations */
struct sg_fd;
-struct sg_request {/* SG_MAX_QUEUE requests outstanding per file */
- struct list_head entry; /* list entry */
- struct sg_fd *parentfp; /* NULL -> not in use */
+/*
+ * For any file descriptor: at any time a sg_request object must be a member
+ * of sg_fd::rq_list or sg_fd::rq_free_list. The only exception is within a
+ * rq_list_lock write lock when it is moving between those two lists.
+ */
+
+struct sg_request {/* active SCSI command or inactive on free list (fl) */
+ struct list_head rq_entry; /* member of rq_list (active cmd) */
+ struct list_head free_entry;/* member of rq_free_list */
+ spinlock_t rq_entry_lck;
struct sg_scatter_hold data;/* hold buffer, perhaps scatter list */
union {
struct sg_io_hdr header; /* see */
- struct sg_io_v4 hdr_v4; /* see */
+ struct sg_v4_hold v4_hold;/* related to */
};
- u8 sense_b[SCSI_SENSE_BUFFERSIZE];
- bool hdr_v4_active; /* selector for anonymous union above */
- bool res_used; /* true -> use reserve buffer, false -> don't */
+ ktime_t start_ts; /* used when sg_fd::time_in_ns is true */
+ enum sg_rq_state rq_state;/* tracks lifetime of each request */
+ bool v4_active; /* selector for autonomous union above */
bool orphan;/* true -> drop on sight, false -> normal */
- bool sg_io_owned; /* true -> packet belongs to SG_IO */
- /* done protected by rq_list_lock */
- char done; /* 0->before bh, 1->before read, 2->read */
+ bool sync_invoc;/* true -> synchronous (e.g. from ioctl(SG_IO)) */
+ u8 sense_b[SCSI_SENSE_BUFFERSIZE];
+ struct sg_fd *parentfp; /* pointer to owning fd, even when on fl */
+ struct sg_scatter_hold *d2p; /* optional 2nd data buffer for bidi */
struct request *rq;
struct bio *bio;
- struct execute_work ew;
+ struct execute_work ew_orph;/* harvest orphan request */
};
-struct sg_fd { /* holds the state of a file descriptor */
- struct list_head sfd_siblings; /* protected by device's sfd_lock */
+struct sg_fd { /* holds the state of a file descriptor */
+ struct list_head sfd_entry; /* member sg_device::sfds list */
struct sg_device *parentdp; /* owning device */
wait_queue_head_t read_wait;/* queue read until command done */
- rwlock_t rq_list_lock; /* protect access to list in req_arr */
struct mutex f_mutex; /* protect against changes in this fd */
+ rwlock_t rq_list_lock; /* protect access to sg_request lists */
+ struct list_head rq_list; /* head of inflight sg_request list */
+ struct list_head rq_free_list; /* head of sg_request free list */
int timeout;/* defaults to SG_DEFAULT_TIMEOUT */
int timeout_user; /* defaults to SG_DEFAULT_TIMEOUT_USER */
- struct sg_scatter_hold reserve; /* one held for this file descriptor */
- struct list_head rq_list; /* head of request list */
- struct fasync_struct *async_qp; /* used by asynchronous notification */
- struct sg_request req_arr[SG_MAX_QUEUE];/* used as singly-linked list */
+ int rem_sgat_thresh;/* > this, request's sgat cleared after use */
+ int tot_fd_thresh; /* E2BIG if sum_of(dlen) > this, 0: ignore */
+ atomic_t sum_fd_dlens; /* when tot_fd_thresh>0 this is sum_of(dlen) */
bool force_packid; /* true -> pack_id input to read() */
bool cmd_q; /* true -> allow command queuing, false -> don't */
- u8 next_cmd_len;/*
[PATCH v3 1/8] sg: types and naming cleanup
Remove typedefs and use better type names like bool and u8 where
appropriate. Rename some variables and functions for clarity.
Adjust formatting (e.g. function definitions) to be more
consistent across the driver.
Signed-off-by: Douglas Gilbert
---
The intention is to move to sg_version_num 40001 when a second
patchset implementing SG_IOSUBMIT and friends is applied. In
the meantime, move from the latest version number in the kernel
(30536) to 30901 to indicate a significant change but not yet
implementing the sg v4 interface.
drivers/scsi/sg.c | 827 ++
1 file changed, 467 insertions(+), 360 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index c6ad00703c5b..78a35e63d177 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -7,7 +7,7 @@
* Original driver (sg.c):
*Copyright (C) 1992 Lawrence Foard
* Version 2 and 3 extensions to driver:
- *Copyright (C) 1998 - 2014 Douglas Gilbert
+ *Copyright (C) 1998 - 2018 Douglas Gilbert
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -16,16 +16,9 @@
*
*/
-static int sg_version_num = 30536; /* 2 digits for each component */
-#define SG_VERSION_STR "3.5.36"
+static int sg_version_num = 30901; /* 2 digits for each component */
+#define SG_VERSION_STR "3.9.01"
-/*
- * D. P. Gilbert (dgilb...@interlog.com), notes:
- * - scsi logging is available via SCSI_LOG_TIMEOUT macros. First
- *the kernel/module needs to be built with CONFIG_SCSI_LOGGING
- *(otherwise the macros compile to empty statements).
- *
- */
#include
#include
@@ -52,6 +45,8 @@ static int sg_version_num = 30536;/* 2 digits for each
component */
#include
#include
#include /* for sg_check_file_access() */
+#include
+#include
#include "scsi.h"
#include
@@ -64,7 +59,7 @@ static int sg_version_num = 30536;/* 2 digits for each
component */
#ifdef CONFIG_SCSI_PROC_FS
#include
-static char *sg_version_date = "20140603";
+static char *sg_version_date = "20181019";
static int sg_proc_init(void);
#endif
@@ -73,7 +68,8 @@ static int sg_proc_init(void);
#define SG_MAX_DEVS 32768
-/* SG_MAX_CDB_SIZE should be 260 (spc4r37 section 3.1.30) however the type
+/*
+ * SG_MAX_CDB_SIZE should be 260 (spc4r37 section 3.1.30) however the type
* of sg_io_hdr::cmd_len can only represent 255. All SCSI commands greater
* than 16 bytes are "variable length" whose length is a multiple of 4
*/
@@ -100,44 +96,54 @@ static int sg_add_device(struct device *, struct
class_interface *);
static void sg_remove_device(struct device *, struct class_interface *);
static DEFINE_IDR(sg_index_idr);
-static DEFINE_RWLOCK(sg_index_lock); /* Also used to lock
- file descriptor list
for device */
+static DEFINE_RWLOCK(sg_index_lock); /* Also used to lock fd list for device */
static struct class_interface sg_interface = {
.add_dev= sg_add_device,
.remove_dev = sg_remove_device,
};
-typedef struct sg_scatter_hold { /* holding area for scsi scatter gather info
*/
- unsigned short k_use_sg; /* Count of kernel scatter-gather pieces */
- unsigned sglist_len; /* size of malloc'd scatter-gather list ++ */
- unsigned bufflen; /* Size of (aggregate) data buffer */
- struct page **pages;
- int page_order;
- char dio_in_use;/* 0->indirect IO (or mmap), 1->dio */
- unsigned char cmd_opcode; /* first byte of command */
-} Sg_scatter_hold;
+struct sg_v4_hold {/* parts of sg_io_v4 object needed in async usage */
+ __user u8 *usr_ptr; /* derived from sg_io_v4::usr_ptr */
+ __user u8 *sbp; /* derived from sg_io_v4::response */
+ u16 cmd_len;/* truncated of sg_io_v4::request_len */
+ u16 max_sb_len; /* truncated of sg_io_v4::max_response_len */
+ u32 flags; /* copy of sg_io_v4::flags */
+};
+
+struct sg_scatter_hold { /* holding area for scsi scatter gather info */
+ struct page **pages;/* num_sgat element array of struct page* */
+ int page_order; /* byte_len = (page_size * (2**page_order)) */
+ int dlen; /* Byte length of data buffer */
+ unsigned short num_sgat;/* actual number of scatter-gather segments */
+ bool dio_in_use;/* false->indirect IO (or mmap), true->dio */
+ u8 cmd_opcode; /* first byte of command */
+};
struct sg_device; /* forward declarations */
struct sg_fd;
-typedef struct sg_request {/* SG_MAX_QUEUE requests outstanding per file */
+struct sg_request {/* SG_MAX_QUEUE requests outstanding per file */
struct list_head entry; /* list entry */
struct sg_fd *parentfp; /* NULL -> not in use */
- Sg_scatter_hold
[PATCH v3 2/8] sg: introduce sg_log macro
Introduce the SG_LOG macro to replace long-winded
'SCSI_LOG_TIMEOUT(3, sg_printk ...' debug messages. Use __func__
wherever appropriate to make the debug messages more portable.
Signed-off-by: Douglas Gilbert
---
Reviewers want SCSI_LOG_* style logging replaced. But with
what? One suggestion is the trace subsystem but that seems
data and event oriented whereas the current logging is call
based with extra messages when error paths are taken. There
are many debugging hours of work crafted into the current
SCSI_LOG_* messages that should not be thrown out for some
pretty solution without additional benefits.
drivers/scsi/sg.c | 162 +-
1 file changed, 72 insertions(+), 90 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 78a35e63d177..94e13a1d21a5 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -216,9 +216,24 @@ static void sg_device_destroy(struct kref *kref);
/* #define SZ_SG_IOVEC sizeof(struct sg_iovec) synonym for 'struct iovec' */
#define SZ_SG_REQ_INFO sizeof(struct sg_req_info)
-#define sg_printk(prefix, sdp, fmt, a...) \
- sdev_prefix_printk(prefix, (sdp)->device, \
- (sdp)->disk->disk_name, fmt, ##a)
+/*
+ * Kernel needs to be built with CONFIG_SCSI_LOGGING to see log messages.
+ * 'depth' is a number between 1 (most severe) and 7 (most noisy, most
+ * information). All messages are logged as informational (KERN_INFO). In
+ * the unexpected situation where sdp is NULL the macro reverts to a pr_info
+ * and ignores CONFIG_SCSI_LOGGING and always prints to the log.
+ */
+#define SG_LOG(depth, sdp, fmt, a...) \
+ do {\
+ if (IS_ERR_OR_NULL(sdp)) { \
+ pr_info("sg: sdp=NULL_or_ERR, " fmt, ##a); \
+ } else {\
+ SCSI_LOG_TIMEOUT(depth, sdev_prefix_printk( \
+KERN_INFO, (sdp)->device, \
+(sdp)->disk->disk_name, fmt, \
+##a)); \
+ } \
+ } while (0)
/*
* The SCSI interfaces that use read() and write() as an asynchronous variant
of
@@ -316,9 +331,8 @@ sg_open(struct inode *inode, struct file *filp)
sdp = sg_get_dev(min_dev);
if (IS_ERR(sdp))
return PTR_ERR(sdp);
-
- SCSI_LOG_TIMEOUT(3, sg_printk(KERN_INFO, sdp,
- "sg_open: flags=0x%x\n", flags));
+ SG_LOG(3, sdp, "%s: flags=0x%x; device open count prior=%d\n",
+ __func__, flags, sdp->open_cnt);
/* This driver's module count bumped by fops_get in */
/* Prevent the device driver from vanishing while we sleep */
@@ -414,9 +428,10 @@ sg_release(struct inode *inode, struct file *filp)
return IS_ERR(sfp) ? PTR_ERR(sfp) : -ENXIO;
}
sdp = sfp->parentdp;
- SCSI_LOG_TIMEOUT(3, sg_printk(KERN_INFO, sdp, "sg_release\n"));
if (IS_ERR_OR_NULL(sdp))
return IS_ERR(sdp) ? PTR_ERR(sdp) : -ENXIO;
+ SG_LOG(3, sdp, "%s: device open count prior=%d\n", __func__,
+ sdp->open_cnt);
mutex_lock(&sdp->open_rel_lock);
scsi_autopm_put_device(sdp->device);
@@ -462,8 +477,7 @@ sg_read(struct file *filp, char __user *buf, size_t count,
loff_t * ppos)
sdp = sfp->parentdp;
if (IS_ERR_OR_NULL(sdp))
return IS_ERR(sdp) ? PTR_ERR(sdp) : -ENXIO;
- SCSI_LOG_TIMEOUT(3, sg_printk(KERN_INFO, sdp, "%s: count=%d\n",
-__func__, (int)count));
+ SG_LOG(3, sdp, "%s: read() count=%d\n", __func__, (int)count);
if (!access_ok(VERIFY_WRITE, buf, count))
return -EFAULT;
@@ -663,10 +677,9 @@ sg_write(struct file *filp, const char __user *buf, size_t
count, loff_t * ppos)
return IS_ERR(sfp) ? PTR_ERR(sfp) : -ENXIO;
}
sdp = sfp->parentdp;
+ SG_LOG(3, sdp, "%s: write(3rd arg) count=%d\n", __func__, (int)count);
if (IS_ERR_OR_NULL(sdp))
return IS_ERR(sdp) ? PTR_ERR(sdp) : -ENXIO;
- SCSI_LOG_TIMEOUT(3, sg_printk(KERN_INFO, sdp, "%s: count=%d\n",
-__func__, (int)count));
if (atomic_read(&sdp->detaching))
return -ENODEV;
if (!((filp->f_flags & O_NONBLOCK) ||
@@ -687,8 +700,7 @@ sg_write(struct file *filp, const char __user *buf, size_t
count, loff_t * ppos)
return -EIO;/* minimum scsi command length is 6 bytes */
if (!(srp = sg_add_request(sfp))) {
- SCSI_LOG_TIMEOUT(1, sg_printk(KERN_INFO, sdp,
- "sg_write: queue
[PATCH v3 0/8] sg: major cleanup, remove max_queue limit
The intention is to add two new ioctls as proposed by Linus Torvalds: SG_IOSUBMIT and SG_IORECEIVE to replace the write()/read() async interface. But first, clean up the driver and remove the SG_MAX_QUEUE limit of no more than 16 queued commands on a file descriptor at a time. A free list has been added and the de-allocation of sg_request objects is deferred until the close() of a file. Locking is extensively reworked, especially at the struct sg_fd and sg_request level. A new SG_SET_GET_EXTENDED ioctl has been added. It allows multiple integer values and booleans to be written to and/or read from the driver. An example is changing and/or reading the reserved request data length (there is one of these per fd). An example of a new feature is changing and/or reading the per-fd upper limit on the sum of outstanding data buffer sizes (default is 16 MB). An example of a boolean is a bit to do all following command timekeeping in nanoseconds rather that the default millseconds. A later patchset will add implementations for the SG_IOSUBMIT and SG_IORECEIVE plus handling of the sg v4 interface with the existing SG_IO ioctl. This patchset is against Martin Petersen's 4.20/scsi-queue branch. ToDo: - work out more modern technique for logging function invocations info with some error paths. Changes since v2: - fix problem with reserve request not being placed on fl - fix locking problem with sg_reserved_sz() - fix sum_fd_dlens counting problem - change new ioctl base numbers back to 0x22 - arrange fl in ascending dlen order apart from dlen=0 entries at tail - SG_GET_REQUEST_TABLE ioctl first fill from active list then, if there is room, it fills from the free list - add SG_SEIRV_DEV_FL_RQS to fetch count of all free list entries on the device the fd is associated with Changes since v1: - remove redundant casts from private_data field - introduce atomic to address locking problems around sg_fd::sum_fd_dlens - replace rq_state defines with an enumeration - add __must_hold() annotation to sg_fill_request_table() - fix compile/build problem around the 4th and 5th patches - add read_value[SG_SEIRV_*] options in SG_SET_GET_EXTENDED ioctl and increase structure size from 64 to 96 bytes Douglas Gilbert (8): sg: types and naming cleanup sg: introduce sg_log macro sg: split header, expand and correct descriptions sg: expand request states sg: add free list, rework locking sg: complete locking changes on ioctl+debug sg: rework ioctl handling sg: user controls for q_at_head, read_value drivers/scsi/sg.c | 2621 ++-- include/scsi/sg.h | 268 +--- include/uapi/scsi/sg.h | 414 +++ 3 files changed, 2156 insertions(+), 1147 deletions(-) create mode 100644 include/uapi/scsi/sg.h -- 2.17.1
Re: [PATCH -next] scsi: hisi_sas: Remove set but not used variable 'dq_list'
On 26/10/2018 08:07, YueHaibing wrote:
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/scsi/hisi_sas/hisi_sas_v1_hw.c: In function 'start_delivery_v1_hw':
drivers/scsi/hisi_sas/hisi_sas_v1_hw.c:907:20: warning:
variable 'dq_list' set but not used [-Wunused-but-set-variable]
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c: In function 'start_delivery_v2_hw':
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c:1671:20: warning:
variable 'dq_list' set but not used [-Wunused-but-set-variable]
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c: In function 'start_delivery_v3_hw':
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c:889:20: warning:
variable 'dq_list' set but not used [-Wunused-but-set-variable]
It never used since introduction in commit
fa222db0b036 ("scsi: hisi_sas: Don't lock DQ for complete task sending")
Signed-off-by: YueHaibing
Acked-by: John Garry
scsi: myrs: warning fix, was: [GIT PULL] first round of SCSI updates for the 4.19+ merge window
On Wed, Oct 24, 2018 at 1:00 PM James Bottomley wrote: > > James Bottomley (1): > scsi: myrs: fix build failure on 32 bit Hi James and Hannes, Since James mentioned 32-bit compiles during the kernel summit, I'd like to confirm that I hit this on my randconfig builder now, with some latency since the last linux-next tree I tested before flying to Edinburgh did not have the bug, and the latest linux-next tree that is available now (dated last Friday) does, and I see your tree is fixed. During normal times, I should catch these within a short time of the patch getting into scsi-next. However, while looking at this bug, I found two more issues related to the specific computation: percent_complete = ldev_info->rbld_lba * 100 / ldev_info->cfg_devsize; I see that both rbld_lba and cfg_devsize are reported by the device, but only the former is 64 bit but the latter is 32 bit and also intended to be the larger of the two. I suspect this is a bug, and the same is also present in the old DAC960.c. cfg_devsize is followed by four reserved bytes in the header, so I suppose it was meant to be 64-bit? If you divide two 64-bit numbers, you also have to use div_u64_64() instead of do_div(). On top of that, I see we get those values from the device but never do any endianess conversion on them. It seems likely that they are all little-endian and require a le32_to_cpu() conversion to also work on big-endian kernel builds. Alternatively we could make the Kconfig symbol as 'depends on !CPU_BIG_ENDIAN || COMPILE_TEST'. Arnd
Re: [PATCH 5/5] qla2xxx: use lower_32_bits and upper_32_bits instead of reinventing them
On Thu, Oct 18, 2018 at 09:43:25PM -0700, Bart Van Assche wrote: > Hi Christoph, > > Have you considered to use put_unaligned_le64() instead of storing the > lower and upper 32 bits separately? I really don't want to touch this old driver all that much, just get rid of the buggy existing helpers.
[PATCH -next] scsi: hisi_sas: Remove set but not used variable 'dq_list'
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/scsi/hisi_sas/hisi_sas_v1_hw.c: In function 'start_delivery_v1_hw':
drivers/scsi/hisi_sas/hisi_sas_v1_hw.c:907:20: warning:
variable 'dq_list' set but not used [-Wunused-but-set-variable]
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c: In function 'start_delivery_v2_hw':
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c:1671:20: warning:
variable 'dq_list' set but not used [-Wunused-but-set-variable]
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c: In function 'start_delivery_v3_hw':
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c:889:20: warning:
variable 'dq_list' set but not used [-Wunused-but-set-variable]
It never used since introduction in commit
fa222db0b036 ("scsi: hisi_sas: Don't lock DQ for complete task sending")
Signed-off-by: YueHaibing
---
drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 2 --
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 2 --
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 2 --
3 files changed, 6 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
index f0e457e..8df822a 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
@@ -904,11 +904,9 @@ static void start_delivery_v1_hw(struct hisi_sas_dq *dq)
{
struct hisi_hba *hisi_hba = dq->hisi_hba;
struct hisi_sas_slot *s, *s1, *s2 = NULL;
- struct list_head *dq_list;
int dlvry_queue = dq->id;
int wp;
- dq_list = &dq->list;
list_for_each_entry_safe(s, s1, &dq->list, delivery) {
if (!s->ready)
break;
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index cc36b64..77a85ea 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -1670,11 +1670,9 @@ static void start_delivery_v2_hw(struct hisi_sas_dq *dq)
{
struct hisi_hba *hisi_hba = dq->hisi_hba;
struct hisi_sas_slot *s, *s1, *s2 = NULL;
- struct list_head *dq_list;
int dlvry_queue = dq->id;
int wp;
- dq_list = &dq->list;
list_for_each_entry_safe(s, s1, &dq->list, delivery) {
if (!s->ready)
break;
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index bd4ce38..a369450 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -886,11 +886,9 @@ static void start_delivery_v3_hw(struct hisi_sas_dq *dq)
{
struct hisi_hba *hisi_hba = dq->hisi_hba;
struct hisi_sas_slot *s, *s1, *s2 = NULL;
- struct list_head *dq_list;
int dlvry_queue = dq->id;
int wp;
- dq_list = &dq->list;
list_for_each_entry_safe(s, s1, &dq->list, delivery) {
if (!s->ready)
break;
--
2.7.0
Re: [PATCH 05/28] IB/srp: remove old request_fn_active check
On 10/25/18 11:10 PM, Jens Axboe wrote:
This check is only viable for non scsi-mq. Since that is going away,
kill this legacy check.
Cc: Bart Van Assche
Cc: Parav Pandit
Cc: linux-scsi@vger.kernel.org
Signed-off-by: Jens Axboe
---
drivers/infiniband/ulp/srp/ib_srp.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c
b/drivers/infiniband/ulp/srp/ib_srp.c
index 0b34e909505f..5a79444c2f3c 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -1334,13 +1334,6 @@ static void srp_terminate_io(struct srp_rport *rport)
struct scsi_device *sdev;
int i, j;
- /*
-* Invoking srp_terminate_io() while srp_queuecommand() is running
-* is not safe. Hence the warning statement below.
-*/
- shost_for_each_device(sdev, shost)
- WARN_ON_ONCE(sdev->request_queue->request_fn_active);
-
for (i = 0; i < target->ch_count; i++) {
ch = &target->ch[i];
I _think_ I've already killed that one; please do check the latest tree
from Doug Ledford.
Cheers,
Hannes
--
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
