On 01/08/2017 08:11 PM, Qu Wenruo wrote: > > > At 01/08/2017 09:16 PM, Goldwyn Rodrigues wrote: >> >> 1. Motivation >> While fixing user space tools for btrfs-progs, I found a couple of bugs >> which are already solved in kernel space but were not ported to user >> space. User space is a little ignored when it comes to fixing bugs in >> the core functionality. XFS developers have already performed this and >> the userspace and kernel code walks in lockstep for libxfs. > > Personally speaking, I'm not a fan of re-using kernel code in btrfs-progs. > > In fact, in btrfs-progs, we don't need a lot of kernel facilities, like > page/VFS/lock(btrfs-progs works in single thread under most case).
The core functionality would be specific to btrfs algorithms. While I agree it cannot do away with kernel components completely, we will be minimizing the dependency of kernel components. The parts which interact with kernel such as inode/dentry/etc handling would be moved out of this core. > > And that should make btrfs-progs easier to maintain. > > Furthermore, there are cases while kernel is doing things wrong while > btrfs-progs does it right. > > Like RAID56 scrub, kernel scrub can screw up P/Q while (still > out-of-tree though) scrub won't. > Time to fix it? > > Personally speaking, I'd like to see btrfs-progs as a lightweight > re-implementation without the mess from kernel. > Btrfs-progs and kernel can do cross-check, while btrfs-progs can be more > developer friendly. This cross-check, and lack of thereof, is what I wish to save. As for being developer-friendly, you need to know the internal workings of the kernel components to work on btrfs-progs. The core shall act as a library on top of which btrfs-progs will be built/migrated. > > (I'm already trying to re-implement btrfs_map_block() in btrfs-progs > with a better interface in out-of-tree offline scrub patchset) > >> >> >> 2. Implementation >> >> 2.1 Code Re-arranaging >> Re-arrange the kernel code so that core functions are in a separate >> directory "libbtrfs" or "core". (There is already libbtrfs_objects >> keyword defined in Makefile.in, so I am not sure if we should use >> libbtrfs, or perhaps just core). The core directory will contain >> algorithms, function prototypes and implementations spcific to btrfs. > > Well, we have kernel-lib/ for it already. > And it sounds much like what you want. While kernel-lib exists, it is not complete to perform a drop-in replacement. > > Further more, there is already work to separate fs/btrfs/ctree.h and > include/uapi/linux/btrfs_tree.h (already done in kernel though). Another advantage of performing this. doing the same thing twice ;) > > We could start from the same thing in btrfs-progs. > >> >> Comparing the current situation, ctree.h is pretty "polluted" with >> functions prototypes which do not belong there, the definition of which >> are not in ctree.c. An example: functions which use struct dentry, inode >> or other kernel component can be moved out of the core. Besides, >> functions which could survive with btrfs_inode as opposed to inode >> should be changed so. We would need new files to split the logic, such >> as creating inode.c to keep all inode related code. >> >> 2.2 Kernel Stubs >> Making the core directory a drop-in replacement will require kernel >> stubs which would make some meaning in user-space. This may or may not >> be included in kerncompat.h. Personally, I would prefer to move >> kerncompat.h into kernel-stub linux/*.h files. The down-side is we could >> have a lot of files and directories for stubs, not forgetting the ones >> for trace/*.h or event asm/*.h. > > Errr, I'm not a fan of 2.2 and 2.3 though. > > Yes, we have cases that over 90% code can be reused from kernel, like > raid6 recovery tables. > > But that's almost pure algorithm part. (And I followed David's > suggestion to put them into kernel-lib) > Yes, it is algorithmic specific code which we want to pull. _Not_ the entire fs/btrfs/*. This would require some additions to kernel-lib and quite a few kernel stubs. > For anything kernel specific, like ftrace/mutex/page/VFS, I prefer a > lightweight re-write, which is easier to write and review. And it should be so. > > In my experience, I just used about 300 lines to rewrite > btrfs_map_block(), compare to kernel 500+ lines and variant wrappers, no > to mention easier to understand interface. > Examples are: > https://patchwork.kernel.org/patch/9488519/ > https://patchwork.kernel.org/patch/9488505/ > > > Especially since there may be more hidden bugs in kernel code. Like Christoph said, We'd rather fix "hidden bugs" in both kernel and tools rather than just the tools. > > Thanks, > Qu > >> >> 2.3 Flag day >> Flag day would be the day we move to the new directory structure. Until >> then, we send the patches with the current directory structure. After >> flag day, all patches must be ported to the new directory structure. We >> could request developers to repost/retest patches leading up to the flag >> day. >> >> >> 3. Post converging >> Checks/scripts to make sure that patches which are pushed to kernel >> space will not render user space tools uncompilable. >> >> While these are my (and my teams) thoughts, I would like suggestions >> and/or criticism to improve the idea. >> > > -- Goldwyn -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
