[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:5.009 / U:0.016 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site