On Wednesday, July 17, 2013 5:44 PM Alan Stern wrote:
> On Wed, 17 Jul 2013, Andrzej Pietrasiewicz wrote:
>
> > Hello Alan,
>
> Hello.
>
> > > > @@ -179,7 +179,7 @@ EXPORT_SYMBOL(fsg_ss_function); void
> > > > fsg_lun_close(struct fsg_lun *curlun) {
> > > > if (curlun->filp) {
> > > > - LDBG(curlun, "close backing file\n");
> > > > + pr_debug("close backing file\n");
> > >
> > > Here and in lots of other places, you have replaced a log message
> > > that includes the LUN's name and driver with a message that includes
> > > neither. If somebody reads the system log after this change, how
> > > are they going to know _which_ LUN had its backing file closed?
> > > That information should be included in the message.
> > >
> >
> > I should have been more explicit in the commit log.
> > My main concern with the LDBG and its friends is that they need to
> > have a device to operate on.
> >
> > So far this hasn't been an issue, because the luns have been
> > represented in sysfs. By the way, using sysfs for this kind of things
> > could be considered "abuse". In a configfs-based gadget the luns are
> > represented in configfs instead, so there is no device to use for
> > dev_dbg and similar; the devices are used in the legacy
> > g_mass_storage.ko, though.
> >
> > As far as I can tell the only reason each lun contains a struct device
> > is for it to appear in sysfs, and the only reason for it to appear in
> > sysfs is to have a file-like attribute for the user to read from/
> > write to. Now that configfs is taking over this responsibility, there
> > is no need for struct device. This is what I think, I guess that
> > Michal's explanation would be authoritative.
> >
> > @Michal: Could you throw some light on the subject?
>
> That's okay; I originally wrote the code that registers LUNs in sysfs.
> You're right; I did it so that there would be file-like attributes for the
> user to access. With configfs the device structures aren't needed.
>
> > Meanwhile I kept the struct device in struct fsg_lun for the sake of
> > Compatibility and it should be kept until the g_xxxx.ko modules are
> > dropped altogether (which might be a long time).
> >
> > > Why not just change the definitions of LDBG, LINFO, and so on
> > > instead of changing all the places where they get called?
> > >
> >
> > If lun's device is to be omitted, there is no point in passing luns to
> > the said macros, which means changing each macro invocation anyway.
> > However, if a lun were useful in the macro your suggestion is
> > absolutely right.
>
> I strongly feel that the macros should print out the LUN's name as a
> prefix to the rest of the message. Indeed, if there are several instances
> of the mass-storage function running (perhaps in multiple interfaces or
> attached to multiple UDCs) then the prefix should also indicate the
> instance the LUN belongs to.
>
> These names don't have to be device names. But they should be sufficient
> to identify the particular LUN the message is about.
>
You are right, I have an idea, I will post a v2 series soon.
Thanks,
Andrzej
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
