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()). -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
