| Message ID | 20200115205912.38688-9-jlayton@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ceph: asynchronous file create support | expand |
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
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.
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
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,
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
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;
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(-)