Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1664205 > unrolled thread

Re: [PATCH] staging: lustre: headers: potential UAPI headers

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] staging: lustre: headers: potential UAPI headers "Dilger, Andreas" <andreas.dilger@intel.com> - 2017-06-12 22:30 +0200
    Re: [PATCH] staging: lustre: headers: potential UAPI headers Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-06-13 06:30 +0200
      Re: [lustre-devel] [PATCH] staging: lustre: headers: potential UAPI  headers James Simmons <jsimmons@infradead.org> - 2017-06-15 18:10 +0200
        Re: [lustre-devel] [PATCH] staging: lustre: headers: potential UAPI  headers Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-06-15 19:00 +0200

#1664205 — Re: [PATCH] staging: lustre: headers: potential UAPI headers

On Jan 21, 2017, at 02:24, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> 
> On Fri, Jan 20, 2017 at 11:33:11PM +0000, James Simmons wrote:
>> 
>>>>> On Mon, Dec 19, 2016 at 12:06:47PM -0500, James Simmons wrote:
>>>>>> Not for landing. This is the purposed UAPI headers
>>>>>> with the removal of unlikely and debugging macros.
>>>>>> This is just for feedback to see if this is acceptable
>>>>>> for the upstream client.
>>>>>> 
>>>>>> Signed-off-by: James Simmons <jsimmons@infradead.org>
>>>>>> ---
>>>>>> .../lustre/lustre/include/lustre/lustre_fid.h      | 353 +++++++++++++++++++++
>>>>>> .../lustre/lustre/include/lustre/lustre_ostid.h    | 233 ++++++++++++++
>>>>> 
>>>>> Can you make a lustre "uapi" directory so we can see which files you
>>>>> really want to be UAPI and which you don't as time goes on?
>>>> 
>>>> Where do you want them placed? In uapi/linux/lustre or uapi/lustre. Does
>>>> it matter to you? The below was to forth coming UAPI headers which from
>>>> your response you seem okay with in general.
>>> 
>>> How many .h files are there going to be?  It's just a single filesystem,
>>> shouldn't you just need a single file?  If so, how about
>>> 	drivers/staging/lustre/include/uapi/lustre.h
>>> ?
>>> 
>>> If you really need multiple .h files, put them all in the same uapi/
>>> directory with a lustre_ prefix, you don't need a whole subdir just for
>>> yourself, right?
>> 
>> We have 12 UAPI headers and yes they all begin with lustre_*. Okay I will
>> create a driver/staging/lustre/include/uapi/linux directory and start
>> moving headers there.
> 
> 12 seems like a lot just for a single, tiny, filesystem :)
> 
> But moving them all to a single directory will see where we can later
> merge them together, sounds like a good plan.

Greg,
are you really adamantly against moving the Lustre headers into their own lustre/
subdirectory?  This change actually breaks userspace libraries/tools that are
using these headers to interface with the kernel (which is the whole point of
the UAPI headers).  Current Lustre tools/libraries include headers like, e.g.:

    #include <lustre/lustre_user.h>

(from /usr/include/lustre) but with this change tools will have to change to use:

    #ifdef HAVE_LINUX_LUSTRE_USER
    #include <linux/lustre_user.h>
    #else
    #include <lustre/lustre_user.h>
    #endif

and will need to have configure checks in the build environment to know which location
to use for include files.  If we put the Lustre headers into an "include/linux/lustre/"
subdir then the tools can continue to use "<lustre/lustre_user.h>" and similar, and
the Makefile can be changed to add "-I /usr/include/linux" (in addition to the normal
"/usr/include") but it will work transparently to the code, regardless of where the
headers are located or which kernel is being used.

On the maintainability point of view, I'd also think that putting headers into a
separate subdir would also be preferable just to avoid /usr/include/uapi/linux from
growing huge.  There are already several storage-related sybsystems that have their
headers in a subdirectory, like ceph, cifs, mtd, nfsd, raid, and sunrpc.

Cheers, Andreas

[toc] | [next] | [standalone]

#1664387

On Mon, Jun 12, 2017 at 08:20:15PM +0000, Dilger, Andreas wrote:
> On Jan 21, 2017, at 02:24, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> > 
> > On Fri, Jan 20, 2017 at 11:33:11PM +0000, James Simmons wrote:
> >> 
> >>>>> On Mon, Dec 19, 2016 at 12:06:47PM -0500, James Simmons wrote:
> >>>>>> Not for landing. This is the purposed UAPI headers
> >>>>>> with the removal of unlikely and debugging macros.
> >>>>>> This is just for feedback to see if this is acceptable
> >>>>>> for the upstream client.
> >>>>>> 
> >>>>>> Signed-off-by: James Simmons <jsimmons@infradead.org>
> >>>>>> ---
> >>>>>> .../lustre/lustre/include/lustre/lustre_fid.h      | 353 +++++++++++++++++++++
> >>>>>> .../lustre/lustre/include/lustre/lustre_ostid.h    | 233 ++++++++++++++
> >>>>> 
> >>>>> Can you make a lustre "uapi" directory so we can see which files you
> >>>>> really want to be UAPI and which you don't as time goes on?

I said that ^^^

> >>>> Where do you want them placed? In uapi/linux/lustre or uapi/lustre. Does
> >>>> it matter to you? The below was to forth coming UAPI headers which from
> >>>> your response you seem okay with in general.
> >>> 
> >>> How many .h files are there going to be?  It's just a single filesystem,
> >>> shouldn't you just need a single file?  If so, how about
> >>> 	drivers/staging/lustre/include/uapi/lustre.h
> >>> ?
> >>> 
> >>> If you really need multiple .h files, put them all in the same uapi/
> >>> directory with a lustre_ prefix, you don't need a whole subdir just for
> >>> yourself, right?
> >> 
> >> We have 12 UAPI headers and yes they all begin with lustre_*. Okay I will
> >> create a driver/staging/lustre/include/uapi/linux directory and start
> >> moving headers there.
> > 
> > 12 seems like a lot just for a single, tiny, filesystem :)
> > 
> > But moving them all to a single directory will see where we can later
> > merge them together, sounds like a good plan.
> 
> Greg,
> are you really adamantly against moving the Lustre headers into their own lustre/
> subdirectory?

I did not say that.

Please, when responding to 5 month old email messages, get your quoting
correct...

greg k-h

[toc] | [prev] | [next] | [standalone]

#1666822 — Re: [lustre-devel] [PATCH] staging: lustre: headers: potential UAPI headers

> On Mon, Jun 12, 2017 at 08:20:15PM +0000, Dilger, Andreas wrote:
> > On Jan 21, 2017, at 02:24, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> > > 
> > > On Fri, Jan 20, 2017 at 11:33:11PM +0000, James Simmons wrote:
> > >> 
> > >>>>> On Mon, Dec 19, 2016 at 12:06:47PM -0500, James Simmons wrote:
> > >>>>>> Not for landing. This is the purposed UAPI headers
> > >>>>>> with the removal of unlikely and debugging macros.
> > >>>>>> This is just for feedback to see if this is acceptable
> > >>>>>> for the upstream client.
> > >>>>>> 
> > >>>>>> Signed-off-by: James Simmons <jsimmons@infradead.org>
> > >>>>>> ---
> > >>>>>> .../lustre/lustre/include/lustre/lustre_fid.h      | 353 +++++++++++++++++++++
> > >>>>>> .../lustre/lustre/include/lustre/lustre_ostid.h    | 233 ++++++++++++++
> > >>>>> 
> > >>>>> Can you make a lustre "uapi" directory so we can see which files you
> > >>>>> really want to be UAPI and which you don't as time goes on?
> 
> I said that ^^^
> 
> > >>>> Where do you want them placed? In uapi/linux/lustre or uapi/lustre. Does
> > >>>> it matter to you? The below was to forth coming UAPI headers which from
> > >>>> your response you seem okay with in general.
> > >>> 
> > >>> How many .h files are there going to be?  It's just a single filesystem,
> > >>> shouldn't you just need a single file?  If so, how about
> > >>> 	drivers/staging/lustre/include/uapi/lustre.h
> > >>> ?
> > >>> 
> > >>> If you really need multiple .h files, put them all in the same uapi/
> > >>> directory with a lustre_ prefix, you don't need a whole subdir just for
> > >>> yourself, right?
> > >> 
> > >> We have 12 UAPI headers and yes they all begin with lustre_*. Okay I will
> > >> create a driver/staging/lustre/include/uapi/linux directory and start
> > >> moving headers there.
> > > 
> > > 12 seems like a lot just for a single, tiny, filesystem :)
> > > 
> > > But moving them all to a single directory will see where we can later
> > > merge them together, sounds like a good plan.
> > 
> > Greg,
> > are you really adamantly against moving the Lustre headers into their own lustre/
> > subdirectory?
> 
> I did not say that.
> 
> Please, when responding to 5 month old email messages, get your quoting
> correct...

So this is coming from trying to understand the "merge them together" 
part. Some people reading this it implies all the headers would be 
eventually merged into one big header and placed into include/uapi/linux. 

We are getting to the point where some sites are migrating from the out
of tree branch to this client. This also means they will be moving 
external open source userland applications shortly. If we expose UAPI 
headers that are completely different that it breaks their tools users
will dump this client and go back to the out source tree. We really like
to avoid that.

[toc] | [prev] | [next] | [standalone]

#1666904 — Re: [lustre-devel] [PATCH] staging: lustre: headers: potential UAPI headers

On Thu, Jun 15, 2017 at 04:48:20PM +0100, James Simmons wrote:
> So this is coming from trying to understand the "merge them together" 
> part. Some people reading this it implies all the headers would be 
> eventually merged into one big header and placed into include/uapi/linux. 

Sounds like a good plan, why wouldn't you want something nice and simple
like that?  :)

> We are getting to the point where some sites are migrating from the out
> of tree branch to this client. This also means they will be moving 
> external open source userland applications shortly. If we expose UAPI 
> headers that are completely different that it breaks their tools users
> will dump this client and go back to the out source tree. We really like
> to avoid that.

Note, as the code is still in staging, files will get renamed and moved
around, you know that, nothing we can do here at the moment.

I have yet to see any patches yet, so I don't know why people are
complaining...

thanks,

greg k-h

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web