[lkml]   [2011]   [Mar]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [git pull] mnt_devname queue
On Thu, Mar 17, 2011 at 03:28:47PM +0800, Xiaotian Feng wrote:
> Cc'ed author ...
> > a) What the hell would you expect to happen if userland mounts it twice
> > and unmount the first one? ??pstore_sb = NULL, pstore_mnt = NULL, AFAICS.

BTW, you want
* mount_single(), not mount_nodev()
* simple_pin_fs()/mntput() around modifying that sucker from kernel
(held across both the file creation and writing to it)

> > b) pstore_writefile() - struct file on stack? ??Really? ??Again, in the
> > scenario above, what'll happen to you if pstore_mnt gets dropped and
> > freed in the middle of all that?
> >
> > c) in the same function:
> > + ?? ?? ?? memset(&f, '0', sizeof f);
> > Ahem...

Aside of a new meaning given to "zero that structure out", why the devil
are you doing it in such a convoluted way? Note that you are using ramfs,
so the mapping is unevictable. Simple kmalloc() + memcpy() and
simple_read_from_buffer() to implement ->read() would do nicely.

And for fsck sake, copy ramfs_get_inode() and trim it. You are overriding
it for the single directory in there and you are only using it for regular
files otherwise. With wrong ->i_op and ->i_fop *and* irrelevant messing with
->i_mapping. Which leaves you with
inode->i_ino = get_next_ino();
inode_init_owner(inode, dir, mode);
inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME;
as useful part.

BTW, you want to make the contents visible before d_add(). You
*definitely* want to finish setting it up before unlocking the parent,
or you are asking for userland to come and unlink() it under you.

Incidentally, you are leaking ->i_private on umount. You want ->evict_inode()
doing that kfree(), not ->unlink(). And possibly ->erase() as well, with
check for zero i_nlink around it (in ->evict_inode()).

 \ /
  Last update: 2011-03-17 11:47    [W:0.070 / U:0.864 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site