Bharath SM <bharathsm.hsk@xxxxxxxxx> writes: > On Mon, Jul 3, 2023 at 9:33 PM Paulo Alcantara <pc@xxxxxxxxxxxxx> wrote: >> >> Bharath SM <bharathsm.hsk@xxxxxxxxx> writes: >> >> > The CREATE_NO_BUFFER and CREATE_WRITE_THROUGH file create options >> > are correctly set in cifs_nt_open and cifs_reopen functions based >> > on O_DIRECT and O_SYNC flags. However flags are missing during the >> > file creation process in cifs_atomic_open, this was leading to >> > successful write operations with O_DIRECT even in case on un-aligned >> > size. Fixed by setting create options based on open file flags. >> > >> > Signed-off-by: Bharath SM <bharathsm@xxxxxxxxxxxxx> >> > --- >> > fs/smb/client/dir.c | 6 ++++++ >> > 1 file changed, 6 insertions(+) >> > >> > diff --git a/fs/smb/client/dir.c b/fs/smb/client/dir.c >> > index 30b1e1bfd204..4178a7fb2ac2 100644 >> > --- a/fs/smb/client/dir.c >> > +++ b/fs/smb/client/dir.c >> > @@ -292,6 +292,12 @@ static int cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned >> > * ACLs >> > */ >> > >> > + if (oflags & O_SYNC) >> > + create_options |= CREATE_WRITE_THROUGH; >> > + >> > + if (oflags & O_DIRECT) >> > + create_options |= CREATE_NO_BUFFER; >> > + >> >> Looks good, thanks. >> >> I see that cifs_nt_open(), cifs_reopen_file() and cifs_do_create() use >> cifs_create_options(). >> >> I'd rather have those flags set in cifs_create_options() by passing a >> new @flags to it, so if we need to make any changes later, only >> cifs_create_options() will require it. What do you think? > > I see there are around 34 places where we call cifs_create_options() > and currently it's taking cifs_sb,create options as arguments. Should > we make another helper function with name > "create_options=cifs_parse_flags(flags)" then pass create_options into > cifs_create_options() ? > I think we can do this cleanup as the next patch. What do you think.? Sounds good. Thanks.
