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? Thanks, 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.?
