[PATCH] ceph: Using file->f_flags rather than flags check O_CREAT.

[PATCH] ceph: Using file->f_flags rather than flags check O_CREAT.

[Ma, Jianpeng]

Because flags removed the O_CREAT, so we should use file->f_flags.

Signed-off-by: Jianpeng Ma <jianpeng.ma@intel.com>
---
 fs/ceph/file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index 8b79d87..e1347cf 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -210,7 +210,7 @@ int ceph_open(struct inode *inode, struct file *file)
        ihold(inode);

        req->r_num_caps = 1;
-       if (flags & O_CREAT)
+       if (file->f_flags & O_CREAT)
                parent_inode = ceph_get_dentry_parent_inode(file->f_path.dentry);
        err = ceph_mdsc_do_request(mdsc, parent_inode, req);
        iput(parent_inode);
--
2.4.3

Re: [PATCH] ceph: Using file->f_flags rather than flags check O_CREAT.

[Yan, Zheng]

On Thu, Aug 13, 2015 at 5:01 PM, Ma, Jianpeng <jianpeng.ma@intel.com> wrote:
> Because flags removed the O_CREAT, so we should use file->f_flags.
>
> Signed-off-by: Jianpeng Ma <jianpeng.ma@intel.com>
> ---
>  fs/ceph/file.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> index 8b79d87..e1347cf 100644
> --- a/fs/ceph/file.c
> +++ b/fs/ceph/file.c
> @@ -210,7 +210,7 @@ int ceph_open(struct inode *inode, struct file *file)
>         ihold(inode);
>
>         req->r_num_caps = 1;
> -       if (flags & O_CREAT)
> +       if (file->f_flags & O_CREAT)
>                 parent_inode = ceph_get_dentry_parent_inode(file->f_path.dentry);
>         err = ceph_mdsc_do_request(mdsc, parent_inode, req);
>         iput(parent_inode);

In this case, we do not need parent_inode because file already exists.
I'd like to remove the code that finds parent inode.


Regards
Yan, Zheng

> --
> 2.4.3
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ceph: Using file->f_flags rather than flags check O_CREAT.

[Sage Weil]

On Thu, 13 Aug 2015, Yan, Zheng wrote:
> On Thu, Aug 13, 2015 at 5:01 PM, Ma, Jianpeng <jianpeng.ma@intel.com> wrote:
> > Because flags removed the O_CREAT, so we should use file->f_flags.
> >
> > Signed-off-by: Jianpeng Ma <jianpeng.ma@intel.com>
> > ---
> >  fs/ceph/file.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> > index 8b79d87..e1347cf 100644
> > --- a/fs/ceph/file.c
> > +++ b/fs/ceph/file.c
> > @@ -210,7 +210,7 @@ int ceph_open(struct inode *inode, struct file *file)
> >         ihold(inode);
> >
> >         req->r_num_caps = 1;
> > -       if (flags & O_CREAT)
> > +       if (file->f_flags & O_CREAT)
> >                 parent_inode = ceph_get_dentry_parent_inode(file->f_path.dentry);
> >         err = ceph_mdsc_do_request(mdsc, parent_inode, req);
> >         iput(parent_inode);
> 
> In this case, we do not need parent_inode because file already exists.
> I'd like to remove the code that finds parent inode.

Do you mean that O_CREAT is only removed in the case where the inode 
already exists?  Because there is at least one case where we do a single 
op to the MDS that does the create + open, and in that case we do need to 
make sure a dir fsync waits for it to commit.  As long as we don't break 
that one!

sage

Re: [PATCH] ceph: Using file->f_flags rather than flags check O_CREAT.

[Yan, Zheng]

On Thu, Aug 13, 2015 at 8:34 PM, Sage Weil <sage@newdream.net> wrote:
> On Thu, 13 Aug 2015, Yan, Zheng wrote:
>> On Thu, Aug 13, 2015 at 5:01 PM, Ma, Jianpeng <jianpeng.ma@intel.com> wrote:
>> > Because flags removed the O_CREAT, so we should use file->f_flags.
>> >
>> > Signed-off-by: Jianpeng Ma <jianpeng.ma@intel.com>
>> > ---
>> >  fs/ceph/file.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/fs/ceph/file.c b/fs/ceph/file.c
>> > index 8b79d87..e1347cf 100644
>> > --- a/fs/ceph/file.c
>> > +++ b/fs/ceph/file.c
>> > @@ -210,7 +210,7 @@ int ceph_open(struct inode *inode, struct file *file)
>> >         ihold(inode);
>> >
>> >         req->r_num_caps = 1;
>> > -       if (flags & O_CREAT)
>> > +       if (file->f_flags & O_CREAT)
>> >                 parent_inode = ceph_get_dentry_parent_inode(file->f_path.dentry);
>> >         err = ceph_mdsc_do_request(mdsc, parent_inode, req);
>> >         iput(parent_inode);
>>
>> In this case, we do not need parent_inode because file already exists.
>> I'd like to remove the code that finds parent inode.
>
> Do you mean that O_CREAT is only removed in the case where the inode
> already exists?  Because there is at least one case where we do a single
> op to the MDS that does the create + open, and in that case we do need to
> make sure a dir fsync waits for it to commit.  As long as we don't break
> that one!
>

create+open case is handled by ceph_atomic_open or ceph_create.
ceph_open is used only when inode already exists.

Regards
Yan, Zheng

> sage