Re: [PATCH 1/3] scsi_dh: Add modalias support for SCSI targets

Chandra Seetharaman
2009-04-03T22:43:08Z

Hi James,

Do you still have any concerns (after Peter's response) ?

If not, Can you accept the patches, please.

Thanks & Regards,

chandra
On Mon, 2009-03-23 at 15:13 -0700, Chandra Seetharaman wrote:
> Hi James,
>
> Are your concerns answered with Peter's response ?
>
> Or, changes are required for the patches I posted ?
>
> Thanks and regards,
>
> chandra
> On Fri, 2009-03-20 at 14:24 -0400, Peter Jones wrote:
> > On 03/19/2009 02:54 PM, Chandra Seetharaman wrote:
> > > On Wed, 2009-03-18 at 12:18 -0700, Chandra Seetharaman wrote:
> > >> On Wed, 2009-03-18 at 19:30 +0100, Kay Sievers wrote:
> > >>> On Wed, Mar 18, 2009 at 02:36, Chandra Seetharaman <sekharan@us.ibm.com> wrote:
> > >>>> From: Peter Jones <pjones@redhat.com>
> > >>>>
> > >>>> This patch allows the use of modaliases on scsi targets to correctly
> > >>>> load scsi device handler modules when the devices are found.
> > >>>> +++ linux-2.6.28/include/linux/mod_devicetable.h
> > >>>> @@ -454,4 +454,10 @@ struct dmi_system_id {
> > >>>>
> > >>>> #define DMI_MATCH(a, b) { a, b }
> > >>>>
> > >>>> +struct scsi_dh_device_id {
> > >>>> + unsigned char type;
> > >>>> + char vendor[9];
> > >>>> + char model[17];
> > >>>> +};
> > >>> Doesn't the static array waste space, when used for the long lists of
> > >>> entries stuffed in arrays of this structure? It will carry a lot of \0
> > >>> chars, and identical strings can not be de-duplicated by the compiler,
> > >>> unlike when pointers are used?
> > >>>
> > >> I had some problems when we used the pointers instead of arrays. Don't
> > >> recall it now.
> > >>
> > >> Will retry and report.
> > >
> > > Hi Kay,
> > >
> > > I tried it and realized modpost is what is giving the problem.
> > >
> > > When the array for vendor and model is changed to pointers, modpost (at
> > > the end of make modules) dies with a segmentation fault.
> > >
> > > Peter, Any details that you can provide ?
> >
> > In this case, the tail wags the dog -- it's a string instead of a pointer basicallly so that the new bits in file2alias.c can look like the other parts of that same file. It's sortof inconvenient for them to be pointers in there...
> >