[RFC,v2,08/10] ceph: add new MDS req field to hold delegated inode number
diff mbox series

Message ID 20200115205912.38688-9-jlayton@kernel.org
State New
Headers show
Series
  • ceph: asynchronous file create support
Related show

Commit Message

Jeff Layton Jan. 15, 2020, 8:59 p.m. UTC
Add new request field to hold the delegated inode number. Encode that
into the message when it's set.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/ceph/mds_client.c | 3 +--
 fs/ceph/mds_client.h | 1 +
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Ilya Dryomov Jan. 17, 2020, 2:47 p.m. UTC | #1
On Wed, Jan 15, 2020 at 9:59 PM Jeff Layton <jlayton@kernel.org> wrote:
>
> Add new request field to hold the delegated inode number. Encode that
> into the message when it's set.
>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/ceph/mds_client.c | 3 +--
>  fs/ceph/mds_client.h | 1 +
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> index e49ca0533df1..b8070e8c4686 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -2466,7 +2466,7 @@ static struct ceph_msg *create_request_message(struct ceph_mds_client *mdsc,
>         head->op = cpu_to_le32(req->r_op);
>         head->caller_uid = cpu_to_le32(from_kuid(&init_user_ns, req->r_uid));
>         head->caller_gid = cpu_to_le32(from_kgid(&init_user_ns, req->r_gid));
> -       head->ino = 0;
> +       head->ino = cpu_to_le64(req->r_deleg_ino);
>         head->args = req->r_args;
>
>         ceph_encode_filepath(&p, end, ino1, path1);
> @@ -2627,7 +2627,6 @@ static int __prepare_send_request(struct ceph_mds_client *mdsc,
>         rhead->flags = cpu_to_le32(flags);
>         rhead->num_fwd = req->r_num_fwd;
>         rhead->num_retry = req->r_attempts - 1;
> -       rhead->ino = 0;
>
>         dout(" r_parent = %p\n", req->r_parent);
>         return 0;
> diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
> index 2a32afa15eb6..0811543ffd79 100644
> --- a/fs/ceph/mds_client.h
> +++ b/fs/ceph/mds_client.h
> @@ -308,6 +308,7 @@ struct ceph_mds_request {
>         int               r_num_fwd;    /* number of forward attempts */
>         int               r_resend_mds; /* mds to resend to next, if any*/
>         u32               r_sent_on_mseq; /* cap mseq request was sent at*/
> +       unsigned long     r_deleg_ino;

u64, as head->ino is __le64?

Thanks,

                Ilya
Jeff Layton Jan. 17, 2020, 4:53 p.m. UTC | #2
On Fri, 2020-01-17 at 15:47 +0100, Ilya Dryomov wrote:
> On Wed, Jan 15, 2020 at 9:59 PM Jeff Layton <jlayton@kernel.org> wrote:
> > Add new request field to hold the delegated inode number. Encode that
> > into the message when it's set.
> > 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >  fs/ceph/mds_client.c | 3 +--
> >  fs/ceph/mds_client.h | 1 +
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> > index e49ca0533df1..b8070e8c4686 100644
> > --- a/fs/ceph/mds_client.c
> > +++ b/fs/ceph/mds_client.c
> > @@ -2466,7 +2466,7 @@ static struct ceph_msg *create_request_message(struct ceph_mds_client *mdsc,
> >         head->op = cpu_to_le32(req->r_op);
> >         head->caller_uid = cpu_to_le32(from_kuid(&init_user_ns, req->r_uid));
> >         head->caller_gid = cpu_to_le32(from_kgid(&init_user_ns, req->r_gid));
> > -       head->ino = 0;
> > +       head->ino = cpu_to_le64(req->r_deleg_ino);
> >         head->args = req->r_args;
> > 
> >         ceph_encode_filepath(&p, end, ino1, path1);
> > @@ -2627,7 +2627,6 @@ static int __prepare_send_request(struct ceph_mds_client *mdsc,
> >         rhead->flags = cpu_to_le32(flags);
> >         rhead->num_fwd = req->r_num_fwd;
> >         rhead->num_retry = req->r_attempts - 1;
> > -       rhead->ino = 0;
> > 
> >         dout(" r_parent = %p\n", req->r_parent);
> >         return 0;
> > diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
> > index 2a32afa15eb6..0811543ffd79 100644
> > --- a/fs/ceph/mds_client.h
> > +++ b/fs/ceph/mds_client.h
> > @@ -308,6 +308,7 @@ struct ceph_mds_request {
> >         int               r_num_fwd;    /* number of forward attempts */
> >         int               r_resend_mds; /* mds to resend to next, if any*/
> >         u32               r_sent_on_mseq; /* cap mseq request was sent at*/
> > +       unsigned long     r_deleg_ino;
> 
> u64, as head->ino is __le64?
> 

Does that actually matter? It should get promoted to 64 bit when we do
the encoding since we're passing by value, and this will never be larger
than 32 bits on a 32 bit box.
Ilya Dryomov Jan. 17, 2020, 5:42 p.m. UTC | #3
On Fri, Jan 17, 2020 at 5:53 PM Jeff Layton <jlayton@kernel.org> wrote:
>
> On Fri, 2020-01-17 at 15:47 +0100, Ilya Dryomov wrote:
> > On Wed, Jan 15, 2020 at 9:59 PM Jeff Layton <jlayton@kernel.org> wrote:
> > > Add new request field to hold the delegated inode number. Encode that
> > > into the message when it's set.
> > >
> > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > ---
> > >  fs/ceph/mds_client.c | 3 +--
> > >  fs/ceph/mds_client.h | 1 +
> > >  2 files changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> > > index e49ca0533df1..b8070e8c4686 100644
> > > --- a/fs/ceph/mds_client.c
> > > +++ b/fs/ceph/mds_client.c
> > > @@ -2466,7 +2466,7 @@ static struct ceph_msg *create_request_message(struct ceph_mds_client *mdsc,
> > >         head->op = cpu_to_le32(req->r_op);
> > >         head->caller_uid = cpu_to_le32(from_kuid(&init_user_ns, req->r_uid));
> > >         head->caller_gid = cpu_to_le32(from_kgid(&init_user_ns, req->r_gid));
> > > -       head->ino = 0;
> > > +       head->ino = cpu_to_le64(req->r_deleg_ino);
> > >         head->args = req->r_args;
> > >
> > >         ceph_encode_filepath(&p, end, ino1, path1);
> > > @@ -2627,7 +2627,6 @@ static int __prepare_send_request(struct ceph_mds_client *mdsc,
> > >         rhead->flags = cpu_to_le32(flags);
> > >         rhead->num_fwd = req->r_num_fwd;
> > >         rhead->num_retry = req->r_attempts - 1;
> > > -       rhead->ino = 0;
> > >
> > >         dout(" r_parent = %p\n", req->r_parent);
> > >         return 0;
> > > diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
> > > index 2a32afa15eb6..0811543ffd79 100644
> > > --- a/fs/ceph/mds_client.h
> > > +++ b/fs/ceph/mds_client.h
> > > @@ -308,6 +308,7 @@ struct ceph_mds_request {
> > >         int               r_num_fwd;    /* number of forward attempts */
> > >         int               r_resend_mds; /* mds to resend to next, if any*/
> > >         u32               r_sent_on_mseq; /* cap mseq request was sent at*/
> > > +       unsigned long     r_deleg_ino;
> >
> > u64, as head->ino is __le64?
> >
>
> Does that actually matter? It should get promoted to 64 bit when we do
> the encoding since we're passing by value, and this will never be larger
> than 32 bits on a 32 bit box.

It raises eyebrows -- one needs to remember that inode numbers
fit into unsigned long when looking at the code.  We are using u64
for inode numbers throughout ceph.ko: ceph_vino, ino parameters to
various functions, etc -- not just in the wire format definitions.
I think sticking to u64 is more clear and consistent.

Thanks,

                Ilya
Jeff Layton Jan. 17, 2020, 6:31 p.m. UTC | #4
On Fri, 2020-01-17 at 18:42 +0100, Ilya Dryomov wrote:
> On Fri, Jan 17, 2020 at 5:53 PM Jeff Layton <jlayton@kernel.org> wrote:
> > On Fri, 2020-01-17 at 15:47 +0100, Ilya Dryomov wrote:
> > > On Wed, Jan 15, 2020 at 9:59 PM Jeff Layton <jlayton@kernel.org> wrote:
> > > > Add new request field to hold the delegated inode number. Encode that
> > > > into the message when it's set.
> > > > 
> > > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > > ---
> > > >  fs/ceph/mds_client.c | 3 +--
> > > >  fs/ceph/mds_client.h | 1 +
> > > >  2 files changed, 2 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> > > > index e49ca0533df1..b8070e8c4686 100644
> > > > --- a/fs/ceph/mds_client.c
> > > > +++ b/fs/ceph/mds_client.c
> > > > @@ -2466,7 +2466,7 @@ static struct ceph_msg *create_request_message(struct ceph_mds_client *mdsc,
> > > >         head->op = cpu_to_le32(req->r_op);
> > > >         head->caller_uid = cpu_to_le32(from_kuid(&init_user_ns, req->r_uid));
> > > >         head->caller_gid = cpu_to_le32(from_kgid(&init_user_ns, req->r_gid));
> > > > -       head->ino = 0;
> > > > +       head->ino = cpu_to_le64(req->r_deleg_ino);
> > > >         head->args = req->r_args;
> > > > 
> > > >         ceph_encode_filepath(&p, end, ino1, path1);
> > > > @@ -2627,7 +2627,6 @@ static int __prepare_send_request(struct ceph_mds_client *mdsc,
> > > >         rhead->flags = cpu_to_le32(flags);
> > > >         rhead->num_fwd = req->r_num_fwd;
> > > >         rhead->num_retry = req->r_attempts - 1;
> > > > -       rhead->ino = 0;
> > > > 
> > > >         dout(" r_parent = %p\n", req->r_parent);
> > > >         return 0;
> > > > diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
> > > > index 2a32afa15eb6..0811543ffd79 100644
> > > > --- a/fs/ceph/mds_client.h
> > > > +++ b/fs/ceph/mds_client.h
> > > > @@ -308,6 +308,7 @@ struct ceph_mds_request {
> > > >         int               r_num_fwd;    /* number of forward attempts */
> > > >         int               r_resend_mds; /* mds to resend to next, if any*/
> > > >         u32               r_sent_on_mseq; /* cap mseq request was sent at*/
> > > > +       unsigned long     r_deleg_ino;
> > > 
> > > u64, as head->ino is __le64?
> > > 
> > 
> > Does that actually matter? It should get promoted to 64 bit when we do
> > the encoding since we're passing by value, and this will never be larger
> > than 32 bits on a 32 bit box.
> 
> It raises eyebrows -- one needs to remember that inode numbers
> fit into unsigned long when looking at the code.  We are using u64
> for inode numbers throughout ceph.ko: ceph_vino, ino parameters to
> various functions, etc -- not just in the wire format definitions.
> I think sticking to u64 is more clear and consistent.
> 

Yeah, now that I think about it, you're right. This is going to be a
inode presented to the MDS, so it will always need to be 64-bit. Fixed
in my tree.

Thanks,
Ilya Dryomov Jan. 20, 2020, 9:41 a.m. UTC | #5
On Fri, Jan 17, 2020 at 7:31 PM Jeff Layton <jlayton@kernel.org> wrote:
>
> On Fri, 2020-01-17 at 18:42 +0100, Ilya Dryomov wrote:
> > On Fri, Jan 17, 2020 at 5:53 PM Jeff Layton <jlayton@kernel.org> wrote:
> > > On Fri, 2020-01-17 at 15:47 +0100, Ilya Dryomov wrote:
> > > > On Wed, Jan 15, 2020 at 9:59 PM Jeff Layton <jlayton@kernel.org> wrote:
> > > > > Add new request field to hold the delegated inode number. Encode that
> > > > > into the message when it's set.
> > > > >
> > > > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > > > ---
> > > > >  fs/ceph/mds_client.c | 3 +--
> > > > >  fs/ceph/mds_client.h | 1 +
> > > > >  2 files changed, 2 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> > > > > index e49ca0533df1..b8070e8c4686 100644
> > > > > --- a/fs/ceph/mds_client.c
> > > > > +++ b/fs/ceph/mds_client.c
> > > > > @@ -2466,7 +2466,7 @@ static struct ceph_msg *create_request_message(struct ceph_mds_client *mdsc,
> > > > >         head->op = cpu_to_le32(req->r_op);
> > > > >         head->caller_uid = cpu_to_le32(from_kuid(&init_user_ns, req->r_uid));
> > > > >         head->caller_gid = cpu_to_le32(from_kgid(&init_user_ns, req->r_gid));
> > > > > -       head->ino = 0;
> > > > > +       head->ino = cpu_to_le64(req->r_deleg_ino);
> > > > >         head->args = req->r_args;
> > > > >
> > > > >         ceph_encode_filepath(&p, end, ino1, path1);
> > > > > @@ -2627,7 +2627,6 @@ static int __prepare_send_request(struct ceph_mds_client *mdsc,
> > > > >         rhead->flags = cpu_to_le32(flags);
> > > > >         rhead->num_fwd = req->r_num_fwd;
> > > > >         rhead->num_retry = req->r_attempts - 1;
> > > > > -       rhead->ino = 0;
> > > > >
> > > > >         dout(" r_parent = %p\n", req->r_parent);
> > > > >         return 0;
> > > > > diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
> > > > > index 2a32afa15eb6..0811543ffd79 100644
> > > > > --- a/fs/ceph/mds_client.h
> > > > > +++ b/fs/ceph/mds_client.h
> > > > > @@ -308,6 +308,7 @@ struct ceph_mds_request {
> > > > >         int               r_num_fwd;    /* number of forward attempts */
> > > > >         int               r_resend_mds; /* mds to resend to next, if any*/
> > > > >         u32               r_sent_on_mseq; /* cap mseq request was sent at*/
> > > > > +       unsigned long     r_deleg_ino;
> > > >
> > > > u64, as head->ino is __le64?
> > > >
> > >
> > > Does that actually matter? It should get promoted to 64 bit when we do
> > > the encoding since we're passing by value, and this will never be larger
> > > than 32 bits on a 32 bit box.
> >
> > It raises eyebrows -- one needs to remember that inode numbers
> > fit into unsigned long when looking at the code.  We are using u64
> > for inode numbers throughout ceph.ko: ceph_vino, ino parameters to
> > various functions, etc -- not just in the wire format definitions.
> > I think sticking to u64 is more clear and consistent.
> >
>
> Yeah, now that I think about it, you're right. This is going to be a
> inode presented to the MDS, so it will always need to be 64-bit. Fixed
> in my tree.

Other patches in this set introduce function parameters and local
variables that are unsigned long and used for inode numbers.  I think
they need to be fixed up as well.

Thanks,

                Ilya

Patch
diff mbox series

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index e49ca0533df1..b8070e8c4686 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -2466,7 +2466,7 @@  static struct ceph_msg *create_request_message(struct ceph_mds_client *mdsc,
 	head->op = cpu_to_le32(req->r_op);
 	head->caller_uid = cpu_to_le32(from_kuid(&init_user_ns, req->r_uid));
 	head->caller_gid = cpu_to_le32(from_kgid(&init_user_ns, req->r_gid));
-	head->ino = 0;
+	head->ino = cpu_to_le64(req->r_deleg_ino);
 	head->args = req->r_args;
 
 	ceph_encode_filepath(&p, end, ino1, path1);
@@ -2627,7 +2627,6 @@  static int __prepare_send_request(struct ceph_mds_client *mdsc,
 	rhead->flags = cpu_to_le32(flags);
 	rhead->num_fwd = req->r_num_fwd;
 	rhead->num_retry = req->r_attempts - 1;
-	rhead->ino = 0;
 
 	dout(" r_parent = %p\n", req->r_parent);
 	return 0;
diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
index 2a32afa15eb6..0811543ffd79 100644
--- a/fs/ceph/mds_client.h
+++ b/fs/ceph/mds_client.h
@@ -308,6 +308,7 @@  struct ceph_mds_request {
 	int               r_num_fwd;    /* number of forward attempts */
 	int               r_resend_mds; /* mds to resend to next, if any*/
 	u32               r_sent_on_mseq; /* cap mseq request was sent at*/
+	unsigned long	  r_deleg_ino;
 
 	struct list_head  r_wait;
 	struct completion r_completion;