All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC] Converging userspace and kernel code
@ 2017-01-08 13:16 Goldwyn Rodrigues
  2017-01-09  2:11 ` Qu Wenruo
  2017-01-10  3:28 ` Anand Jain
  0 siblings, 2 replies; 19+ messages in thread
From: Goldwyn Rodrigues @ 2017-01-08 13:16 UTC (permalink / raw)
  To: linux-btrfs, Jeff Mahoney, David Sterba


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.


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.

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.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] Converging userspace and kernel code
  2017-01-08 13:16 [RFC] Converging userspace and kernel code Goldwyn Rodrigues
@ 2017-01-09  2:11 ` Qu Wenruo
  2017-01-09  9:31   ` Christoph Hellwig
                     ` (2 more replies)
  2017-01-10  3:28 ` Anand Jain
  1 sibling, 3 replies; 19+ messages in thread
From: Qu Wenruo @ 2017-01-09  2:11 UTC (permalink / raw)
  To: Goldwyn Rodrigues, linux-btrfs, Jeff Mahoney, David Sterba



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).

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.


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.

(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.

Further more, there is already work to separate fs/btrfs/ctree.h and 
include/uapi/linux/btrfs_tree.h (already done in kernel though).

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)

For anything kernel specific, like ftrace/mutex/page/VFS, I prefer a 
lightweight re-write, which is easier to write and review.

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.

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.
>



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] Converging userspace and kernel code
  2017-01-09  2:11 ` Qu Wenruo
@ 2017-01-09  9:31   ` Christoph Hellwig
  2017-01-09 12:06   ` Goldwyn Rodrigues
  2017-01-09 15:31   ` Eric Sandeen
  2 siblings, 0 replies; 19+ messages in thread
From: Christoph Hellwig @ 2017-01-09  9:31 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: Goldwyn Rodrigues, linux-btrfs, Jeff Mahoney, David Sterba

On Mon, Jan 09, 2017 at 10:11:29AM +0800, Qu Wenruo wrote:
> (I'm already trying to re-implement btrfs_map_block() in btrfs-progs with a
> better interface in out-of-tree offline scrub patchset)

How about doing the right thing in the kernel as well?  While looking
at the btrfs abuse of the block REQ_* flag I had to read and understand
__btrfs_map_block and agree it's ripe for refactoring.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] Converging userspace and kernel code
  2017-01-09  2:11 ` Qu Wenruo
  2017-01-09  9:31   ` Christoph Hellwig
@ 2017-01-09 12:06   ` Goldwyn Rodrigues
  2017-01-10  0:35     ` Qu Wenruo
  2017-01-09 15:31   ` Eric Sandeen
  2 siblings, 1 reply; 19+ messages in thread
From: Goldwyn Rodrigues @ 2017-01-09 12:06 UTC (permalink / raw)
  To: Qu Wenruo, linux-btrfs, Jeff Mahoney, David Sterba



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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] Converging userspace and kernel code
  2017-01-09  2:11 ` Qu Wenruo
  2017-01-09  9:31   ` Christoph Hellwig
  2017-01-09 12:06   ` Goldwyn Rodrigues
@ 2017-01-09 15:31   ` Eric Sandeen
  2017-01-09 21:34     ` Omar Sandoval
  2 siblings, 1 reply; 19+ messages in thread
From: Eric Sandeen @ 2017-01-09 15:31 UTC (permalink / raw)
  To: Qu Wenruo, Goldwyn Rodrigues, linux-btrfs, Jeff Mahoney, David Sterba

On 1/8/17 8: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.

But it already does re-use kernel code, it's just that the re-use is
extremely stale, with unfixed bugs in both directions as a result
(at least last time I looked.)

> 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).
> 
> And that should make btrfs-progs easier to maintain.

But as Goldwyn already pointed out, many bugs have gone un-fixed
in userspace, in code which was forked long ago from kernelspace.

For things like locking it's trivial to define that away.

xfsprogs does i.e. -

/* miscellaneous kernel routines not in user space */
#define down_read(a)            ((void) 0)
#define up_read(a)              ((void) 0)
#define spin_lock_init(a)       ((void) 0)
#define spin_lock(a)            ((void) 0)
#define spin_unlock(a)          ((void) 0)
#define likely(x)               (x)
#define unlikely(x)             (x)
#define rcu_read_lock()         ((void) 0)
#define rcu_read_unlock()       ((void) 0)
#define WARN_ON_ONCE(expr)      ((void) 0)


> Furthermore, there are cases while kernel is doing things wrong while
> btrfs-progs does it right.

All the more reason to sync it up, fixes should always be in both
places, right?

I had looked at this a few years ago, and started trying to sync things
up, but got daunted and busy and never completed anything.  :(  I sent
a few fixups back in April 2013 to get things /slightly/ closer.

The libxfs sync in xfs has borne fruit; I'm of the opinion that similar
work would help btrfs too, though it can be a long road.

(e2fsprogs has gone the other way, and has a completely separate
re-implementation in userspace; it works, I guess, but I have to say
that I really like the code commonality in xfs.)

Thanks,
-Eric


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] Converging userspace and kernel code
  2017-01-09 15:31   ` Eric Sandeen
@ 2017-01-09 21:34     ` Omar Sandoval
  2017-01-09 21:38       ` Jeff Mahoney
  0 siblings, 1 reply; 19+ messages in thread
From: Omar Sandoval @ 2017-01-09 21:34 UTC (permalink / raw)
  To: Eric Sandeen
  Cc: Qu Wenruo, Goldwyn Rodrigues, linux-btrfs, Jeff Mahoney, David Sterba

On Mon, Jan 09, 2017 at 09:31:39AM -0600, Eric Sandeen wrote:
> On 1/8/17 8: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.
> 
> But it already does re-use kernel code, it's just that the re-use is
> extremely stale, with unfixed bugs in both directions as a result
> (at least last time I looked.)
> 
> > 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).
> > 
> > And that should make btrfs-progs easier to maintain.
> 
> But as Goldwyn already pointed out, many bugs have gone un-fixed
> in userspace, in code which was forked long ago from kernelspace.
> 
> For things like locking it's trivial to define that away.
> 
> xfsprogs does i.e. -
> 
> /* miscellaneous kernel routines not in user space */
> #define down_read(a)            ((void) 0)
> #define up_read(a)              ((void) 0)
> #define spin_lock_init(a)       ((void) 0)
> #define spin_lock(a)            ((void) 0)
> #define spin_unlock(a)          ((void) 0)
> #define likely(x)               (x)
> #define unlikely(x)             (x)
> #define rcu_read_lock()         ((void) 0)
> #define rcu_read_unlock()       ((void) 0)
> #define WARN_ON_ONCE(expr)      ((void) 0)
> 
> 
> > Furthermore, there are cases while kernel is doing things wrong while
> > btrfs-progs does it right.
> 
> All the more reason to sync it up, fixes should always be in both
> places, right?
> 
> I had looked at this a few years ago, and started trying to sync things
> up, but got daunted and busy and never completed anything.  :(  I sent
> a few fixups back in April 2013 to get things /slightly/ closer.
> 
> The libxfs sync in xfs has borne fruit; I'm of the opinion that similar
> work would help btrfs too, though it can be a long road.
> 
> (e2fsprogs has gone the other way, and has a completely separate
> re-implementation in userspace; it works, I guess, but I have to say
> that I really like the code commonality in xfs.)
> 

Yup, I also think we should be going in the XFS direction. It's a big
maintenance burden to have to worry about the code in two places. (E.g.,
the biggest reason I haven't gotten around to implementing full free
space tree support in btrfs-progs is that it's such a pain in the ass to
port new kernel code to the outdated progs code.)

As far as I know, the only reason it hasn't happened yet is that no one
has agreed to do it, and we're all just hoping someone else will take
care of it.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] Converging userspace and kernel code
  2017-01-09 21:34     ` Omar Sandoval
@ 2017-01-09 21:38       ` Jeff Mahoney
  2017-01-10  1:46         ` Darrick J. Wong
  0 siblings, 1 reply; 19+ messages in thread
From: Jeff Mahoney @ 2017-01-09 21:38 UTC (permalink / raw)
  To: Omar Sandoval, Eric Sandeen
  Cc: Qu Wenruo, Goldwyn Rodrigues, linux-btrfs, David Sterba


[-- Attachment #1.1: Type: text/plain, Size: 3745 bytes --]

On 1/9/17 4:34 PM, Omar Sandoval wrote:
> On Mon, Jan 09, 2017 at 09:31:39AM -0600, Eric Sandeen wrote:
>> On 1/8/17 8: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.
>>
>> But it already does re-use kernel code, it's just that the re-use is
>> extremely stale, with unfixed bugs in both directions as a result
>> (at least last time I looked.)
>>
>>> 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).
>>>
>>> And that should make btrfs-progs easier to maintain.
>>
>> But as Goldwyn already pointed out, many bugs have gone un-fixed
>> in userspace, in code which was forked long ago from kernelspace.
>>
>> For things like locking it's trivial to define that away.
>>
>> xfsprogs does i.e. -
>>
>> /* miscellaneous kernel routines not in user space */
>> #define down_read(a)            ((void) 0)
>> #define up_read(a)              ((void) 0)
>> #define spin_lock_init(a)       ((void) 0)
>> #define spin_lock(a)            ((void) 0)
>> #define spin_unlock(a)          ((void) 0)
>> #define likely(x)               (x)
>> #define unlikely(x)             (x)
>> #define rcu_read_lock()         ((void) 0)
>> #define rcu_read_unlock()       ((void) 0)
>> #define WARN_ON_ONCE(expr)      ((void) 0)
>>
>>
>>> Furthermore, there are cases while kernel is doing things wrong while
>>> btrfs-progs does it right.
>>
>> All the more reason to sync it up, fixes should always be in both
>> places, right?
>>
>> I had looked at this a few years ago, and started trying to sync things
>> up, but got daunted and busy and never completed anything.  :(  I sent
>> a few fixups back in April 2013 to get things /slightly/ closer.
>>
>> The libxfs sync in xfs has borne fruit; I'm of the opinion that similar
>> work would help btrfs too, though it can be a long road.
>>
>> (e2fsprogs has gone the other way, and has a completely separate
>> re-implementation in userspace; it works, I guess, but I have to say
>> that I really like the code commonality in xfs.)
>>
> 
> Yup, I also think we should be going in the XFS direction. It's a big
> maintenance burden to have to worry about the code in two places. (E.g.,
> the biggest reason I haven't gotten around to implementing full free
> space tree support in btrfs-progs is that it's such a pain in the ass to
> port new kernel code to the outdated progs code.)

Another advantage is that we will be able to at least write test cases
for the shared code that can be run entirely in userspace.  That
obviously doesn't address a huge class of potential problems, but it's
better than what we have now.

> As far as I know, the only reason it hasn't happened yet is that no one
> has agreed to do it, and we're all just hoping someone else will take
> care of it.

Yeah, I think that's exactly it.  Having one shared source base is going
to be easier to maintain for everyone.  The longer we wait to do it, the
more it will diverge.  This is a topic we've been discussing in our
team's weekly call for the past few weeks.  I think Goldwyn is granting
that last wish. :)

-Jeff

-- 
Jeff Mahoney
SUSE Labs


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 841 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] Converging userspace and kernel code
  2017-01-09 12:06   ` Goldwyn Rodrigues
@ 2017-01-10  0:35     ` Qu Wenruo
  2017-01-10  0:56       ` Omar Sandoval
  0 siblings, 1 reply; 19+ messages in thread
From: Qu Wenruo @ 2017-01-10  0:35 UTC (permalink / raw)
  To: Goldwyn Rodrigues, linux-btrfs, Jeff Mahoney, David Sterba



At 01/09/2017 08:06 PM, Goldwyn Rodrigues wrote:
>
>
> 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.

If we can move kernel facilities completely (or at least most of them), 
then the idea sounds good for me.

So the first step would be isolating btrfs algorithms from kernel 
facilities.

>
>>
>> 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?

Patch already sent.

BTW, in kernel and in btrfs-progs, scrub are completely different.
Kernel scrub uses different flags and have tons of dirty hacks for 
variants fixes.

While in btrfs-progs offline scrub, everything is just as easy as 
reading things out, check csum and parity.

>
>>
>> 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.

That depends on how independent the "core" btrfs part is.

If completely independent,(Don't ever has any page/VFS/race code) I'm 
completely fine with that.

But the problem is, at least from current code base, even btrfs_path 
structure has quite a lot kernel facilities, mainly locks.
(Just check the different size of btrfs_path in kernel and btrfs-progs)

So although the idea itself is very nice, but the practice may be quite 
hard.

That's why I prefer current btrfs-progs method, because it's more close 
to the core of btrfs algorithm, unlike kernel, which is a superset.
In this case, I prefer to let kernel use and expand btrfs-progs code, 
other than pull in a superset from kernel.

>
>>
>> (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.

While most of the bugs I found in RAID5/6 and dev-replace are all 
related to race, which doesn't ever exist in btrfs-progs.
(That's why I love btrfs-progs so much than the kernel part)

So the problem is still how independent we can extract the core functions.

Thanks,
Qu

>
>>
>> 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.
>>>
>>
>>
>



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] Converging userspace and kernel code
  2017-01-10  0:35     ` Qu Wenruo
@ 2017-01-10  0:56       ` Omar Sandoval
  0 siblings, 0 replies; 19+ messages in thread
From: Omar Sandoval @ 2017-01-10  0:56 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: Goldwyn Rodrigues, linux-btrfs, Jeff Mahoney, David Sterba

On Tue, Jan 10, 2017 at 08:35:26AM +0800, Qu Wenruo wrote:
> That depends on how independent the "core" btrfs part is.
> 
> If completely independent,(Don't ever has any page/VFS/race code) I'm
> completely fine with that.
> 
> But the problem is, at least from current code base, even btrfs_path
> structure has quite a lot kernel facilities, mainly locks.
> (Just check the different size of btrfs_path in kernel and btrfs-progs)
> 
> So although the idea itself is very nice, but the practice may be quite
> hard.
> 
> That's why I prefer current btrfs-progs method, because it's more close to
> the core of btrfs algorithm, unlike kernel, which is a superset.
> In this case, I prefer to let kernel use and expand btrfs-progs code, other
> than pull in a superset from kernel.

[snip]

> While most of the bugs I found in RAID5/6 and dev-replace are all related to
> race, which doesn't ever exist in btrfs-progs.
> (That's why I love btrfs-progs so much than the kernel part)
> 
> So the problem is still how independent we can extract the core functions.
> 
> Thanks,
> Qu

I think you're getting too hung up on scrub/device replace. It doesn't
have to be an all-or-nothing thing. If the scrub code is too insane or
broken to pull in, then we can probably get away with a clean
reimplementation in btrfs-progs if we really want to. I think for a
majority of the Btrfs code, locking and other kernel-specific stuff
won't be too distracting. A good starting point might be the core B-tree
code.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] Converging userspace and kernel code
  2017-01-09 21:38       ` Jeff Mahoney
@ 2017-01-10  1:46         ` Darrick J. Wong
  2017-01-10  2:24           ` Qu Wenruo
  0 siblings, 1 reply; 19+ messages in thread
From: Darrick J. Wong @ 2017-01-10  1:46 UTC (permalink / raw)
  To: Jeff Mahoney
  Cc: Omar Sandoval, Eric Sandeen, Qu Wenruo, Goldwyn Rodrigues,
	linux-btrfs, David Sterba

On Mon, Jan 09, 2017 at 04:38:22PM -0500, Jeff Mahoney wrote:
> On 1/9/17 4:34 PM, Omar Sandoval wrote:
> > On Mon, Jan 09, 2017 at 09:31:39AM -0600, Eric Sandeen wrote:
> >> On 1/8/17 8: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.

Eh, I've wrangled the two other FSes mentioned, so I guess I can
reiterate for a while too. :P

Yes, it's very nice to be able to apply kernel patches of core libxfs
code onto xfsprogs and have it work.  It's /annoying/ to have to it sort
of work but with a lot of fuzz and somewhere midway through the apply
loop the whole thing crashes and burns due to some minor merge conflict.

(It's not so bad for libext2fs since at least it's a totally different
implementation.  But I'll get to that later.)

The core XFS algorithms (btrees, space management, inodes, directories,
attributes, on-disk formats, and some of the log stuff) live in
libxfs/xfs_*.[ch].   The stuff that sits between those algorithms and
the kernel all live in fs/xfs/*.[ch], and the stuff between the
algorithms and the C library live in libxfs/ and include/ in files that
don't start with "xfs_".  As I understand it, the dividing line is that
core algorithms are in libxfs, and everything else isn't.

> >>> Personally speaking, I'm not a fan of re-using kernel code in
> >>> btrfs-progs.
> >>
> >> But it already does re-use kernel code, it's just that the re-use is
> >> extremely stale, with unfixed bugs in both directions as a result
> >> (at least last time I looked.)
> >>
> >>> 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).
> >>>
> >>> And that should make btrfs-progs easier to maintain.

The way I look at it is that there's core algorithms and on-disk format
stuff that can be the same wherever it is, and this code ought to be
kept in sync so that bugfixes and features end up the same in both.
The 'core algorithms' library can talk to the same interfaces in the
kernel and userspace, even though the implementations are different, or,
as Eric points out, even #define'd away.

> >> But as Goldwyn already pointed out, many bugs have gone un-fixed
> >> in userspace, in code which was forked long ago from kernelspace.

Ick.

> >> For things like locking it's trivial to define that away.
> >>
> >> xfsprogs does i.e. -
> >>
> >> /* miscellaneous kernel routines not in user space */
> >> #define down_read(a)            ((void) 0)
> >> #define up_read(a)              ((void) 0)
> >> #define spin_lock_init(a)       ((void) 0)
> >> #define spin_lock(a)            ((void) 0)
> >> #define spin_unlock(a)          ((void) 0)
> >> #define likely(x)               (x)
> >> #define unlikely(x)             (x)
> >> #define rcu_read_lock()         ((void) 0)
> >> #define rcu_read_unlock()       ((void) 0)
> >> #define WARN_ON_ONCE(expr)      ((void) 0)
> >>
> >>
> >>> Furthermore, there are cases while kernel is doing things wrong while
> >>> btrfs-progs does it right.
> >>
> >> All the more reason to sync it up, fixes should always be in both
> >> places, right?
> >>
> >> I had looked at this a few years ago, and started trying to sync things
> >> up, but got daunted and busy and never completed anything.  :(  I sent
> >> a few fixups back in April 2013 to get things /slightly/ closer.
> >>
> >> The libxfs sync in xfs has borne fruit; I'm of the opinion that similar
> >> work would help btrfs too, though it can be a long road.
> >>
> >> (e2fsprogs has gone the other way, and has a completely separate
> >> re-implementation in userspace; it works, I guess, but I have to say
> >> that I really like the code commonality in xfs.)

Having worked on ext* also I will say that as far as finding and
resolving ambiguities in the on-disk specification, having two
independent and maintained implementations of ext4 is wonderful.

Unfortunately, it's /really/ expensive to engineer both, and there are
plenty of small behavioral discrepancies between the two.  In your free
time, run xfstests against fuse2fs and regular ext4. ;)

> > Yup, I also think we should be going in the XFS direction. It's a big
> > maintenance burden to have to worry about the code in two places. (E.g.,
> > the biggest reason I haven't gotten around to implementing full free
> > space tree support in btrfs-progs is that it's such a pain in the ass to
> > port new kernel code to the outdated progs code.)

Yeah.  That sucked for the (very) few times I had to do that for
xfsprogs.

> Another advantage is that we will be able to at least write test cases
> for the shared code that can be run entirely in userspace.  That
> obviously doesn't address a huge class of potential problems, but it's
> better than what we have now.
> 
> > As far as I know, the only reason it hasn't happened yet is that no one
> > has agreed to do it, and we're all just hoping someone else will take
> > care of it.
> 
> Yeah, I think that's exactly it.  Having one shared source base is going
> to be easier to maintain for everyone.  The longer we wait to do it, the
> more it will diverge.  This is a topic we've been discussing in our
> team's weekly call for the past few weeks.  I think Goldwyn is granting
> that last wish. :)

I hope you succeed.

--D

> 
> -Jeff
> 
> -- 
> Jeff Mahoney
> SUSE Labs
> 




^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] Converging userspace and kernel code
  2017-01-10  1:46         ` Darrick J. Wong
@ 2017-01-10  2:24           ` Qu Wenruo
  0 siblings, 0 replies; 19+ messages in thread
From: Qu Wenruo @ 2017-01-10  2:24 UTC (permalink / raw)
  To: Darrick J. Wong, Jeff Mahoney
  Cc: Omar Sandoval, Eric Sandeen, Goldwyn Rodrigues, linux-btrfs,
	David Sterba



At 01/10/2017 09:46 AM, Darrick J. Wong wrote:
> On Mon, Jan 09, 2017 at 04:38:22PM -0500, Jeff Mahoney wrote:
>> On 1/9/17 4:34 PM, Omar Sandoval wrote:
>>> On Mon, Jan 09, 2017 at 09:31:39AM -0600, Eric Sandeen wrote:
>>>> On 1/8/17 8: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.
>
> Eh, I've wrangled the two other FSes mentioned, so I guess I can
> reiterate for a while too. :P
>
> Yes, it's very nice to be able to apply kernel patches of core libxfs
> code onto xfsprogs and have it work.  It's /annoying/ to have to it sort
> of work but with a lot of fuzz and somewhere midway through the apply
> loop the whole thing crashes and burns due to some minor merge conflict.
>
> (It's not so bad for libext2fs since at least it's a totally different
> implementation.  But I'll get to that later.)
>
> The core XFS algorithms (btrees, space management, inodes, directories,
> attributes, on-disk formats, and some of the log stuff) live in
> libxfs/xfs_*.[ch].   The stuff that sits between those algorithms and
> the kernel all live in fs/xfs/*.[ch], and the stuff between the
> algorithms and the C library live in libxfs/ and include/ in files that
> don't start with "xfs_".  As I understand it, the dividing line is that
> core algorithms are in libxfs, and everything else isn't.

Well, thanks for pointing out libxfs, which I didn't ever know before.

After a quick glance, I must say, this is AWESOME!!!

Over 55K lines of codes, but kernel facilities are kept into minimal!
No mutex/spinlock, and VFS inode/page seldom occurs.

What a wonderful implementation!
This is almost a perfect extraction, making core logical really isolated 
from all this kernel mess.
(On the other hand, btrfs is just a hell of random/ancient fixes)

If btrfs can do it like this(although I still wonder), then I'm 
completely OK for that.

While from what I see in current codes like btrfs_map_block(), I still 
wonder if we can do it without a large code/interface rework.
(But personally speaking, I'm quite happy if we can do a large rework, 
quite a lot interfaces are just insane and hard to understand)

Thanks,
Qu

>
>>>>> Personally speaking, I'm not a fan of re-using kernel code in
>>>>> btrfs-progs.
>>>>
>>>> But it already does re-use kernel code, it's just that the re-use is
>>>> extremely stale, with unfixed bugs in both directions as a result
>>>> (at least last time I looked.)
>>>>
>>>>> 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).
>>>>>
>>>>> And that should make btrfs-progs easier to maintain.
>
> The way I look at it is that there's core algorithms and on-disk format
> stuff that can be the same wherever it is, and this code ought to be
> kept in sync so that bugfixes and features end up the same in both.
> The 'core algorithms' library can talk to the same interfaces in the
> kernel and userspace, even though the implementations are different, or,
> as Eric points out, even #define'd away.
>
>>>> But as Goldwyn already pointed out, many bugs have gone un-fixed
>>>> in userspace, in code which was forked long ago from kernelspace.
>
> Ick.
>
>>>> For things like locking it's trivial to define that away.
>>>>
>>>> xfsprogs does i.e. -
>>>>
>>>> /* miscellaneous kernel routines not in user space */
>>>> #define down_read(a)            ((void) 0)
>>>> #define up_read(a)              ((void) 0)
>>>> #define spin_lock_init(a)       ((void) 0)
>>>> #define spin_lock(a)            ((void) 0)
>>>> #define spin_unlock(a)          ((void) 0)
>>>> #define likely(x)               (x)
>>>> #define unlikely(x)             (x)
>>>> #define rcu_read_lock()         ((void) 0)
>>>> #define rcu_read_unlock()       ((void) 0)
>>>> #define WARN_ON_ONCE(expr)      ((void) 0)
>>>>
>>>>
>>>>> Furthermore, there are cases while kernel is doing things wrong while
>>>>> btrfs-progs does it right.
>>>>
>>>> All the more reason to sync it up, fixes should always be in both
>>>> places, right?
>>>>
>>>> I had looked at this a few years ago, and started trying to sync things
>>>> up, but got daunted and busy and never completed anything.  :(  I sent
>>>> a few fixups back in April 2013 to get things /slightly/ closer.
>>>>
>>>> The libxfs sync in xfs has borne fruit; I'm of the opinion that similar
>>>> work would help btrfs too, though it can be a long road.
>>>>
>>>> (e2fsprogs has gone the other way, and has a completely separate
>>>> re-implementation in userspace; it works, I guess, but I have to say
>>>> that I really like the code commonality in xfs.)
>
> Having worked on ext* also I will say that as far as finding and
> resolving ambiguities in the on-disk specification, having two
> independent and maintained implementations of ext4 is wonderful.
>
> Unfortunately, it's /really/ expensive to engineer both, and there are
> plenty of small behavioral discrepancies between the two.  In your free
> time, run xfstests against fuse2fs and regular ext4. ;)
>
>>> Yup, I also think we should be going in the XFS direction. It's a big
>>> maintenance burden to have to worry about the code in two places. (E.g.,
>>> the biggest reason I haven't gotten around to implementing full free
>>> space tree support in btrfs-progs is that it's such a pain in the ass to
>>> port new kernel code to the outdated progs code.)
>
> Yeah.  That sucked for the (very) few times I had to do that for
> xfsprogs.
>
>> Another advantage is that we will be able to at least write test cases
>> for the shared code that can be run entirely in userspace.  That
>> obviously doesn't address a huge class of potential problems, but it's
>> better than what we have now.
>>
>>> As far as I know, the only reason it hasn't happened yet is that no one
>>> has agreed to do it, and we're all just hoping someone else will take
>>> care of it.
>>
>> Yeah, I think that's exactly it.  Having one shared source base is going
>> to be easier to maintain for everyone.  The longer we wait to do it, the
>> more it will diverge.  This is a topic we've been discussing in our
>> team's weekly call for the past few weeks.  I think Goldwyn is granting
>> that last wish. :)
>
> I hope you succeed.
>
> --D
>
>>
>> -Jeff
>>
>> --
>> Jeff Mahoney
>> SUSE Labs
>>
>
>
>
>
>



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] Converging userspace and kernel code
  2017-01-08 13:16 [RFC] Converging userspace and kernel code Goldwyn Rodrigues
  2017-01-09  2:11 ` Qu Wenruo
@ 2017-01-10  3:28 ` Anand Jain
  2017-01-10 12:14   ` Goldwyn Rodrigues
  1 sibling, 1 reply; 19+ messages in thread
From: Anand Jain @ 2017-01-10  3:28 UTC (permalink / raw)
  To: Goldwyn Rodrigues, linux-btrfs, Jeff Mahoney, David Sterba


Goldwyn,

  Could you add a list what functionality in btrfs-progs will
  be using the 'core'. ?

Thanks, Anand


On 01/08/17 21:16, 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.
>
>
> 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.
>
> 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.
>
> 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.
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] Converging userspace and kernel code
  2017-01-10  3:28 ` Anand Jain
@ 2017-01-10 12:14   ` Goldwyn Rodrigues
  2017-01-10 15:20     ` Anand Jain
  0 siblings, 1 reply; 19+ messages in thread
From: Goldwyn Rodrigues @ 2017-01-10 12:14 UTC (permalink / raw)
  To: Anand Jain, linux-btrfs, Jeff Mahoney, David Sterba



On 01/09/2017 09:28 PM, Anand Jain wrote:
> 
> Goldwyn,
> 
>  Could you add a list what functionality in btrfs-progs will
>  be using the 'core'. ?


There are too many to list. It would contain the algorithmic functions
of btrfs which would be able to interact with both kernel and btrfs-progs.



> 
> Thanks, Anand
> 
> 
> On 01/08/17 21:16, 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.
>>
>>
>> 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.
>>
>> 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.
>>
>> 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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] Converging userspace and kernel code
  2017-01-10 12:14   ` Goldwyn Rodrigues
@ 2017-01-10 15:20     ` Anand Jain
  2017-01-10 16:04       ` Goldwyn Rodrigues
  0 siblings, 1 reply; 19+ messages in thread
From: Anand Jain @ 2017-01-10 15:20 UTC (permalink / raw)
  To: Goldwyn Rodrigues, linux-btrfs, Jeff Mahoney, David Sterba



On 01/10/17 20:14, Goldwyn Rodrigues wrote:
>
>
> On 01/09/2017 09:28 PM, Anand Jain wrote:
>>
>> Goldwyn,
>>
>>  Could you add a list what functionality in btrfs-progs will
>>  be using the 'core'. ?
>
>
> There are too many to list. It would contain the algorithmic functions
> of btrfs which would be able to interact with both kernel and btrfs-progs.

  I am getting confused. How about a few from the btrfs-progs ?

Thanks, Anand


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] Converging userspace and kernel code
  2017-01-10 15:20     ` Anand Jain
@ 2017-01-10 16:04       ` Goldwyn Rodrigues
  2017-01-11  2:23         ` Anand Jain
  0 siblings, 1 reply; 19+ messages in thread
From: Goldwyn Rodrigues @ 2017-01-10 16:04 UTC (permalink / raw)
  To: Anand Jain, linux-btrfs, Jeff Mahoney, David Sterba



On 01/10/2017 09:20 AM, Anand Jain wrote:
> 
> 
> On 01/10/17 20:14, Goldwyn Rodrigues wrote:
>>
>>
>> On 01/09/2017 09:28 PM, Anand Jain wrote:
>>>
>>> Goldwyn,
>>>
>>>  Could you add a list what functionality in btrfs-progs will
>>>  be using the 'core'. ?
>>
>>
>> There are too many to list. It would contain the algorithmic functions
>> of btrfs which would be able to interact with both kernel and
>> btrfs-progs.
> 
>  I am getting confused. How about a few from the btrfs-progs ?
> 
> 

Functions such as open_ctree(), or the set/get functions. The idea is to
keep the codebase of this core component the same in btrfs-progs and kernel.

As an example, see how XFS have done using libxfs. Something around the
same lines.

-- 
Goldwyn

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] Converging userspace and kernel code
  2017-01-10 16:04       ` Goldwyn Rodrigues
@ 2017-01-11  2:23         ` Anand Jain
  2017-01-11  2:32           ` Qu Wenruo
  2017-01-11  2:55           ` Goldwyn Rodrigues
  0 siblings, 2 replies; 19+ messages in thread
From: Anand Jain @ 2017-01-11  2:23 UTC (permalink / raw)
  To: Goldwyn Rodrigues, linux-btrfs, Jeff Mahoney, David Sterba



On 01/11/17 00:04, Goldwyn Rodrigues wrote:
>
>
> On 01/10/2017 09:20 AM, Anand Jain wrote:
>>
>>
>> On 01/10/17 20:14, Goldwyn Rodrigues wrote:
>>>
>>>
>>> On 01/09/2017 09:28 PM, Anand Jain wrote:
>>>>
>>>> Goldwyn,
>>>>
>>>>  Could you add a list what functionality in btrfs-progs will
>>>>  be using the 'core'. ?
>>>
>>>
>>> There are too many to list. It would contain the algorithmic functions
>>> of btrfs which would be able to interact with both kernel and
>>> btrfs-progs.
>>
>>  I am getting confused. How about a few from the btrfs-progs ?
>>
>>
>
> Functions such as open_ctree(), or the set/get functions. The idea is to
> keep the codebase of this core component the same in btrfs-progs and kernel.

  The btrfs-progs open_ctree() is used for the offline functionality
  such as fsck...

> As an example, see how XFS have done using libxfs. Something around the
> same lines.
>

  Does XFS progs access the disks directly (without going through the
  kernel) while kernel has mounted the disk ? If yes, how does it
  maintain the consistency ? Calling sync for the sake of read access
  by the progs, is not a good idea as it causes jitters in the steady
  state IOPS for the applications. I have investigated these concerns
  at the data centers earlier.

Thanks, Anand

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] Converging userspace and kernel code
  2017-01-11  2:23         ` Anand Jain
@ 2017-01-11  2:32           ` Qu Wenruo
  2017-01-11  2:55           ` Goldwyn Rodrigues
  1 sibling, 0 replies; 19+ messages in thread
From: Qu Wenruo @ 2017-01-11  2:32 UTC (permalink / raw)
  To: Anand Jain, Goldwyn Rodrigues, linux-btrfs, Jeff Mahoney, David Sterba



At 01/11/2017 10:23 AM, Anand Jain wrote:
>
>
> On 01/11/17 00:04, Goldwyn Rodrigues wrote:
>>
>>
>> On 01/10/2017 09:20 AM, Anand Jain wrote:
>>>
>>>
>>> On 01/10/17 20:14, Goldwyn Rodrigues wrote:
>>>>
>>>>
>>>> On 01/09/2017 09:28 PM, Anand Jain wrote:
>>>>>
>>>>> Goldwyn,
>>>>>
>>>>>  Could you add a list what functionality in btrfs-progs will
>>>>>  be using the 'core'. ?
>>>>
>>>>
>>>> There are too many to list. It would contain the algorithmic functions
>>>> of btrfs which would be able to interact with both kernel and
>>>> btrfs-progs.
>>>
>>>  I am getting confused. How about a few from the btrfs-progs ?
>>>
>>>
>>
>> Functions such as open_ctree(), or the set/get functions. The idea is to
>> keep the codebase of this core component the same in btrfs-progs and
>> kernel.
>
>  The btrfs-progs open_ctree() is used for the offline functionality
>  such as fsck...
>
>> As an example, see how XFS have done using libxfs. Something around the
>> same lines.
>>
>
>  Does XFS progs access the disks directly (without going through the
>  kernel) while kernel has mounted the disk ? If yes, how does it
>  maintain the consistency ? Calling sync for the sake of read access
>  by the progs, is not a good idea as it causes jitters in the steady
>  state IOPS for the applications. I have investigated these concerns
>  at the data centers earlier.

Just a quick glance, and I found that libxfs is much like interfaces of 
OOP languages.

For example, in libxfs, there are functions calling xfs_trans_commit(), 
but there is not implementation.

The implementation part can diff between kernel and xfsprogs.


Which seems very nicely arranged for me, and since it's just interfaces, 
I hardly see how it can cause performance problems.

Thanks,
Qu
>
> Thanks, Anand
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] Converging userspace and kernel code
  2017-01-11  2:23         ` Anand Jain
  2017-01-11  2:32           ` Qu Wenruo
@ 2017-01-11  2:55           ` Goldwyn Rodrigues
  2017-01-11 10:58             ` Anand Jain
  1 sibling, 1 reply; 19+ messages in thread
From: Goldwyn Rodrigues @ 2017-01-11  2:55 UTC (permalink / raw)
  To: Anand Jain, linux-btrfs, Jeff Mahoney, David Sterba



On 01/10/2017 08:23 PM, Anand Jain wrote:
> 
> 
> On 01/11/17 00:04, Goldwyn Rodrigues wrote:
>>
>>
>> On 01/10/2017 09:20 AM, Anand Jain wrote:
>>>
>>>
>>> On 01/10/17 20:14, Goldwyn Rodrigues wrote:
>>>>
>>>>
>>>> On 01/09/2017 09:28 PM, Anand Jain wrote:
>>>>>
>>>>> Goldwyn,
>>>>>
>>>>>  Could you add a list what functionality in btrfs-progs will
>>>>>  be using the 'core'. ?
>>>>
>>>>
>>>> There are too many to list. It would contain the algorithmic functions
>>>> of btrfs which would be able to interact with both kernel and
>>>> btrfs-progs.
>>>
>>>  I am getting confused. How about a few from the btrfs-progs ?
>>>
>>>
>>
>> Functions such as open_ctree(), or the set/get functions. The idea is to
>> keep the codebase of this core component the same in btrfs-progs and
>> kernel.
> 
>  The btrfs-progs open_ctree() is used for the offline functionality
>  such as fsck...
> 
>> As an example, see how XFS have done using libxfs. Something around the
>> same lines.
>>
> 
>  Does XFS progs access the disks directly (without going through the
>  kernel) while kernel has mounted the disk ? If yes, how does it
>  maintain the consistency ? Calling sync for the sake of read access
>  by the progs, is not a good idea as it causes jitters in the steady
>  state IOPS for the applications. I have investigated these concerns
>  at the data centers earlier.
> 

I think you are misunderstanding the effort. The effort is not to use
the common code simultaneously between the kernel and the progs, but to
keep a common *codebase*. Both kernel and userspace would provide the
same functionality as it is currently, though independently in their own
right. IOW, this is not a data sync but a codebase sync.

The objective (besides others) is to have faster bug resolution, and
better "transfer" of logic from kernel to user space and vice versa.

xfsprogs, depending on the operation used acts on the disk directly
without the kernel mounting it (say mkfs or xfs_repair), or disk being
mounted (say xfs_growfs)


-- 
Goldwyn

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] Converging userspace and kernel code
  2017-01-11  2:55           ` Goldwyn Rodrigues
@ 2017-01-11 10:58             ` Anand Jain
  0 siblings, 0 replies; 19+ messages in thread
From: Anand Jain @ 2017-01-11 10:58 UTC (permalink / raw)
  To: Goldwyn Rodrigues, linux-btrfs, Jeff Mahoney, David Sterba




>>>>>> Goldwyn,
>>>>>>
>>>>>>  Could you add a list what functionality in btrfs-progs will
>>>>>>  be using the 'core'. ?
>>>>>
>>>>>
>>>>> There are too many to list. It would contain the algorithmic functions
>>>>> of btrfs which would be able to interact with both kernel and
>>>>> btrfs-progs.
>>>>
>>>>  I am getting confused. How about a few from the btrfs-progs ?
>>>>
>>>>
>>>
>>> Functions such as open_ctree(), or the set/get functions. The idea is to
>>> keep the codebase of this core component the same in btrfs-progs and
>>> kernel.
>>
>>  The btrfs-progs open_ctree() is used for the offline functionality
>>  such as fsck...
>>
>>> As an example, see how XFS have done using libxfs. Something around the
>>> same lines.
>>>
>>
>>  Does XFS progs access the disks directly (without going through the
>>  kernel) while kernel has mounted the disk ? If yes, how does it
>>  maintain the consistency ? Calling sync for the sake of read access
>>  by the progs, is not a good idea as it causes jitters in the steady
>>  state IOPS for the applications. I have investigated these concerns
>>  at the data centers earlier.
>>
>
> I think you are misunderstanding the effort. The effort is not to use
> the common code simultaneously between the kernel and the progs, but to
> keep a common *codebase*. Both kernel and userspace would provide the
> same functionality as it is currently, though independently in their own
> right. IOW, this is not a data sync but a codebase sync.
>
> The objective (besides others) is to have faster bug resolution, and
> better "transfer" of logic from kernel to user space and vice versa.
>
> xfsprogs, depending on the operation used acts on the disk directly
> without the kernel mounting it (say mkfs or xfs_repair), or disk being
> mounted (say xfs_growfs)

  Thanks for clarifying. Once we had progs and kernel simultaneous
  disk access. Its been fixed now. Good, that idea is not coming
  back with this.

  Indeed this a nice effort to have it organized.

  Just a note: fsck, which is a strong consumer of open_ctree() in
  the unmounted mode, should go obsolete per the plan. Its an area
  which needs some help though.

Thanks, Anand


^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2017-01-11 10:55 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-08 13:16 [RFC] Converging userspace and kernel code Goldwyn Rodrigues
2017-01-09  2:11 ` Qu Wenruo
2017-01-09  9:31   ` Christoph Hellwig
2017-01-09 12:06   ` Goldwyn Rodrigues
2017-01-10  0:35     ` Qu Wenruo
2017-01-10  0:56       ` Omar Sandoval
2017-01-09 15:31   ` Eric Sandeen
2017-01-09 21:34     ` Omar Sandoval
2017-01-09 21:38       ` Jeff Mahoney
2017-01-10  1:46         ` Darrick J. Wong
2017-01-10  2:24           ` Qu Wenruo
2017-01-10  3:28 ` Anand Jain
2017-01-10 12:14   ` Goldwyn Rodrigues
2017-01-10 15:20     ` Anand Jain
2017-01-10 16:04       ` Goldwyn Rodrigues
2017-01-11  2:23         ` Anand Jain
2017-01-11  2:32           ` Qu Wenruo
2017-01-11  2:55           ` Goldwyn Rodrigues
2017-01-11 10:58             ` Anand Jain

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.