Re: dmfs for 2.5.51

[Greg KH <greg@kroah.com>]

On Fri, Dec 13, 2002 at 09:58:06AM +0000, Joe Thornber wrote:
> On Thu, Dec 12, 2002 at 09:25:51PM -0800, Greg KH wrote:
> > On Thu, Dec 12, 2002 at 05:50:01PM -0800, Andrew Morton wrote:
> > > hm.  The whole thing seems hokey to me.  Not sure why.
> > 
> > I agree.  It doesn't feel right.  I mean, doing a mkdir(1) to create a
> > device, which causes files to be created automagically in that
> > directory?  Something needs to change here, and I proposed a single file
> > to write to that creates the device, but was shot down by the author.
> 
> Greg, I didn't mean to make it sound like I was shooting you down, I
> just said that we'd leave it as it was for now.

Sorry, I didn't mean it that way.  I understand and was trying to reach
more people.  Looks like I succeded :)

> Having written the
> code I wanted a bit more feedback.  When I started writing the fs
> interface a couple of people expressed concerns that I should try and
> map things properly onto fs semantics and not just create a single
> file.  Given Andrews comment I guess I haven't done a good job.  Could
> you flesh out your single file idea a bit more please, there's a lot
> of functionality to shoe horn in there.

Ok, I'll go work on that and see how it turns out.

> > > > ...
> > > > +  echo -e "0 56 linear /dev/hda3 0\n56 102344 linear /dev/hda4 0" > table
> > > 
> > > Maybe this is why.
> > 
> > Heh, yeah, welcome to parsers in the kernel :)
> > But the dm code today does much the same thing with ioctls, passing a
> > string down to the loaded modules below it.  So there is a bit of
> > president.  Even if it is ugly :)
> 
> y, the dm targets have always accepted their arguments as ascii
> strings.  So the file system interface just adds code to split the
> input into lines, and then sscanf the first three elements of the line
> - this is probably less code than the marshalling of binary data that
> is done in the ioctl interface.
> 
> I see the fact that we're using ascii data as being the big advantage
> of the fs interface, which neccessarily means doing a small amount of
> parsing in kernel.  You can't have things both ways.

I agree, I like the interface.  It's just a little strange the first
time you see it.

> > > > ...
> > > > +static struct page *find_page(struct dmfs_file *f, loff_t len, int fill)
> > > 
> > > This is called under spinlock.
> > > 
> > > > ...
> > > > +                       void *addr = (void *) __get_free_page(GFP_KERNEL);
> > > 
> > > whoops.
> 
> My fault :(
> 
> > Nice catch.  I'm not sure that the find_page(), __io() and friends
> > functions are really needed at all.
> 
> It would be nice to get rid of them, what shall we replace them with ?

Something like only creating one page as I did in the dev file, or the
seq_file interface.  I'll play around with the sizes of the files to see
how to fix this up.

thanks,

greg k-h