On Tue, 2 Apr 2013 16:48:05 -0700
Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> On Tue, Apr 02, 2013 at 04:33:57PM -0700, Jacob Pan wrote:
> > On Tue, 2 Apr 2013 16:00:42 -0700
> > Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> >
> > > > +#include "intel_rapl.h"
> > > > +#include "../../../fs/sysfs/sysfs.h"
> > >
> > > WTF?
> > >
> > > Oh, that's a sure sign you are not doing something properly, if
> > > you think it's ok to muck around with the internals of sysfs.
> > >
> > > There's a reason that file is "private", why do you think it's ok
> > > to use it directly? Did you just think that I somehow "forgot"
> > > to put it in the proper include directory?
> > I did feel unsure about this but i saw some precedence in the
> > kernel.
>
> Someone else is doing this with the sysfs api? I don't see any other
> code in Linus's tree doing this at the moment, where did you see this?
> Let me know and I'll fix it up right away.
>
no, i did not mean sysfs api. I mean include internal header files via
#include ../../
e.g.in drivers/usb/image/microtek.c
#include "../../scsi/scsi.h"
#include <scsi/scsi_host.h>
> > Anyway, I needed a way to validate a userspace file passed to rapl
> > driver belong to the same sysfs directory. I will look for
> > alternative ways.
>
> What do you mean by this? What exactly are you trying to do? No
> normal driver code should _ever_ call sysfs functions directly, nor
> should they ever care about sysfs internals.
>
i did not call sysfs internal calls, just need to use
struct sysfs_dirent {}
to do the following sanity check against user passed event control file,
it is still not a 100% strong check.
/* check if the cfile belongs to the same rapl domain */
if (strcmp(rd->kobj.sd->s_name,
cfile->f_dentry->d_parent->d_name.name)) {
pr_debug("cfile does not belong to domain %s\n",
rd->kobj.sd->s_name);
ret = -EINVAL;
goto exit_cleanup_fds;
}
> And, odds are, you didn't test your code as a module, right, as any
> internal sysfs function that you could get from this .h file, wouldn't
> be exported for a module to use, unless I missed one somewhere?
>
I did run the driver as module since i didn't use sysfs internal
functions, just the struct. I may be hitting a corner case here, but
for drivers who need to discover sysfs hierarchy would it be useful to
expose some info in struct sysfs_dirent{}?
> greg k-h
> --
> To unsubscribe from this list: send the line "unsubscribe
> platform-driver-x86" in the body of a message to
> majordomo@xxxxxxxxxxxxxxx More majordomo info at
> http://vger.kernel.org/majordomo-info.html
[Jacob Pan]
--
Thanks,
Jacob
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
