On Thu, Sep 28, 2006 at 01:32:46PM -0700, Andrew Morton wrote:
> > +obj-$(CONFIG_SCSI) += scsi_wait_scan.o
> > +
> > scsi_mod-y += scsi.o hosts.o scsi_ioctl.o constants.o \
> > scsicam.o scsi_error.o scsi_lib.o \
> > scsi_scan.o scsi_sysfs.o \
>
> I think that's supposed to go into scsi_mod-y, at line 146.
Nope. If CONFIG_SCSI is a module, we want this module around so people
can insmod it in order to ensure that scsi discovery is complete. If
CONFIG_SCSI is built-in, we want this built-in too.
> > +static char scsi_scan_type[6] = "sync";
>
> That wasted a byte.
If you specify "async" then the buffer needs to be 6 bytes long.
> >
> > +static spinlock_t async_scan_lock = SPIN_LOCK_UNLOCKED;
>
> DEFINE_SPINLOCK()
OK.
> > +int scsi_complete_async_scans(void)
> > +{
> > + struct async_scan_data *data;
> > +
> > + do {
> > + if (list_empty(&scanning_hosts))
> > + return 0;
> > + data = kmalloc(sizeof(*data), GFP_KERNEL);
> > + if (!data)
> > + msleep(1);
> > + } while (!data);
>
> ick. Immortal allocation loops are poor form. If there's really no
> alternative, please use __GFP_NOFAIL, so people can easily grep for your
> sins.
It's not immortal. The scanning threads will eventually terminate, and
so will this loop.
> > + data->shost = NULL;
> > + init_completion(&data->prev_finished);
> > +
> > + spin_lock(&async_scan_lock);
> > + if (list_empty(&scanning_hosts))
> > + goto done;
> > + list_add_tail(&data->list, &scanning_hosts);
> > + spin_unlock(&async_scan_lock);
>
> "If the list is not empty, stick something on it".
>
> What an unusual thing to do. I'm sure it makes sense, but some comments
> explaining what's going on would be nice.
OK.
> > +#ifdef MODULE
> > +/* Only exported for the benefit of scsi_wait_scan */
> > +EXPORT_SYMBOL_GPL(scsi_complete_async_scans);
> > +#endif
>
> Is that actually needed? AFACIT this .o file will just get linked against
> the one which contains scsi_wait_scan() anyway.
See above.
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
