linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jan Harkes <jaharkes@cs.cmu.edu>
To: Yann Droneaud <ydroneaud@opteya.com>
Cc: David Howells <dhowells@redhat.com>,
	linux-api@vger.kernel.org, linux-kbuild@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 05/11] UAPI: coda: Don't use internal kernel structs in UAPI
Date: Thu, 6 Sep 2018 08:16:24 -0400	[thread overview]
Message-ID: <20180906121623.6lrt6lajqstdymtx@cs.cmu.edu> (raw)
In-Reply-To: <2e337313415a1ede1fb7a777902a0de7cb7c2602.camel@opteya.com>

On Thu, Sep 06, 2018 at 01:52:29PM +0200, Yann Droneaud wrote:
> Hi,
> 
> Le jeudi 06 septembre 2018 � 08:13 +0100, David Howells a �crit :
> > Yann Droneaud <ydroneaud@opteya.com> wrote:
> > 
> > > This structure should not have been exposed to userspace in the
> > > first
> > > place: it's unusable by userspace as it is. It was incorrect to
> > > have it
> > > outside of #ifdef __KERNEL__ before commit 607ca46e97a1b ... 
> > > ...
> > > All CODA_REQ_* defines internals to kernel side and not exchanged
> > > with
> > > userspace.
> > > 
> > > Please move them back to <linux/coda_psdev.h>
> > 
> > Is there any reason coda_psdev.h needs to be in include/linux/ rather
> > than fs/coda/?
> > 
> 
> It's a valid concern.
> 
> At first I thought the first lines (see below) could have been useful
> for userspace:
> 
>   #define CODA_PSDEV_MAJOR 67
>   #define MAX_CODADEVS  5    /* how many do we allow */

Nope, userspace just tries to open /dev/cfs0, or a manually configured
alternative. We have only used linux/coda.h, and actually carry our own
copy of that file which is kept in sync manually, which is why there are
all those ifdefs for different systems in there. This all originates
from the time of the 2.1.x kernels when Coda was built externally.

> But the file was unsuable for a long long time so we can assume it's
> usage by userspace is deprecated, then we could remove it from UAPI,
> and moves its content back to include/linux.
> 
> As one could see include/linux/coda_psdev.h is not used outside of
> fs/coda, moving the header here as you suggests seems to be the correct
> solution.

Agreed.

Jan

  reply	other threads:[~2018-09-06 16:51 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-05 15:54 [RFC] UAPI: Check headers by compiling all together as C++ David Howells
2018-09-05 15:55 ` [PATCH 05/11] UAPI: coda: Don't use internal kernel structs in UAPI David Howells
2018-09-05 16:54   ` Jan Harkes
2018-09-05 17:12   ` Yann Droneaud
2018-09-05 17:28     ` Jan Harkes
2018-09-05 17:24   ` David Howells
2018-09-06  7:13   ` David Howells
2018-09-06 11:52     ` Yann Droneaud
2018-09-06 12:16       ` Jan Harkes [this message]
2018-09-06 14:53     ` David Howells
2018-09-05 15:55 ` [PATCH 07/11] UAPI: nilfs2: Fix use of undefined byteswapping functions David Howells
2018-09-05 22:20   ` Al Viro
2018-09-05 16:55 ` [RFC] UAPI: Check headers by compiling all together as C++ Greg KH
2018-09-05 17:33   ` Yann Droneaud
2018-09-05 17:42     ` Michael S. Tsirkin
2018-09-06  7:12     ` Yann Droneaud
2018-09-05 19:22   ` Jan Engelhardt
2018-09-05 17:50 ` David Howells

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180906121623.6lrt6lajqstdymtx@cs.cmu.edu \
    --to=jaharkes@cs.cmu.edu \
    --cc=dhowells@redhat.com \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ydroneaud@opteya.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).