[illumos-Developer] webrev: 1282 nsmb_open has a race

[Gordon Ross]

On Wed, Jul 27, 2011 at 5:02 PM, Garrett D'Amore <garrett at damore.org> wrote:
> Instead of
>  279         case DDI_INFO_DEVT2INSTANCE:
>  280                 *result = NULL;
>
> I would prefer *result = nsmb_dip ? ddi_get_instance(nsmb_dip) : NULL;

Why that preference?  Several other pseudo drivers do this, i.e.
  io/log.c io/ksyms.c io/sysevent.c

> Better yet, why not simply decode the minor number, like this:
>
>        ddi_get_minor(dev);

The major can vary too, in theory.  It's tricky code.

The code to get major+minor was originally copied from...
well, I forget which place, exactly, but here's an example:
  http://bugs.ec.illumos.org:8080/source/xref/illumos-gate/usr/src/uts/common/fs/zfs/zfs_vfsops.c#194

> (On that note, using soft state, like you had before, but dereferencing
> the soft state by minor number, seems like the more clean solution,
> rather than relying on the only one instance rule.)

I'm not sure I understand this suggestion.
It is getting the soft state by minor, right?
That's in nsmb_open, not attach.  It's here:
 http://bugs.ec.illumos.org:8080/source/xref/illumos-gate/usr/src/uts/common/fs/smbclnt/netsmb/smb_dev.c#480

The code I took out (getting a softc for unit zero)
has nothing to do with the normal code path in this
driver where it gets a (per-unit) softc object.

Would appreciate clarifications.

Thanks,
Gordon