Re: [PATCH 1/2] CIFS: Simplify non-posix open stuff (try #2)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2010/12/8 Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>:
> On Wed, Dec 8, 2010 at 8:28 AM, Pavel Shilovsky <piastryyy@xxxxxxxxx> wrote:
>> Delete cifs_open_inode_helper and move non-posix open related things
>> to cifs_nt_open function.
>>
>> Signed-off-by: Pavel Shilovsky <piastryyy@xxxxxxxxx>
>> ---
>>  fs/cifs/file.c |  189 ++++++++++++++++++++++----------------------------------
>>  1 files changed, 73 insertions(+), 116 deletions(-)
>>
>> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
>> index b857ce5..8f6a084 100644
>> --- a/fs/cifs/file.c
>> +++ b/fs/cifs/file.c
>> @@ -104,53 +104,6 @@ static inline int cifs_get_disposition(unsigned int flags)
>>                return FILE_OPEN;
>>  }
>>
>> -static inline int cifs_open_inode_helper(struct inode *inode,
>> -       struct cifsTconInfo *pTcon, __u32 oplock, FILE_ALL_INFO *buf,
>> -       char *full_path, int xid)
>> -{
>> -       struct cifsInodeInfo *pCifsInode = CIFS_I(inode);
>> -       struct timespec temp;
>> -       int rc;
>> -
>> -       if (pCifsInode->clientCanCacheRead) {
>> -               /* we have the inode open somewhere else
>> -                  no need to discard cache data */
>> -               goto client_can_cache;
>> -       }
>> -
>> -       /* BB need same check in cifs_create too? */
>> -       /* if not oplocked, invalidate inode pages if mtime or file
>> -          size changed */
>> -       temp = cifs_NTtimeToUnix(buf->LastWriteTime);
>> -       if (timespec_equal(&inode->i_mtime, &temp) &&
>> -                          (inode->i_size ==
>> -                           (loff_t)le64_to_cpu(buf->EndOfFile))) {
>> -               cFYI(1, "inode unchanged on server");
>> -       } else {
>> -               if (inode->i_mapping) {
>> -                       /* BB no need to lock inode until after invalidate
>> -                       since namei code should already have it locked? */
>> -                       rc = filemap_write_and_wait(inode->i_mapping);
>> -                       mapping_set_error(inode->i_mapping, rc);
>> -               }
>> -               cFYI(1, "invalidating remote inode since open detected it "
>> -                        "changed");
>> -               invalidate_remote_inode(inode);
>> -       }
>> -
>> -client_can_cache:
>> -       if (pTcon->unix_ext)
>> -               rc = cifs_get_inode_info_unix(&inode, full_path, inode->i_sb,
>> -                                             xid);
>> -       else
>> -               rc = cifs_get_inode_info(&inode, full_path, buf, inode->i_sb,
>> -                                        xid, NULL);
>> -
>> -       cifs_set_oplock_level(pCifsInode, oplock);
>> -
>> -       return rc;
>> -}
>> -
>>  int cifs_posix_open(char *full_path, struct inode **pinode,
>>                        struct super_block *sb, int mode, unsigned int f_flags,
>>                        __u32 *poplock, __u16 *pnetfid, int xid)
>> @@ -213,6 +166,76 @@ posix_open_ret:
>>        return rc;
>>  }
>>
>> +static int
>> +cifs_nt_open(char *full_path, struct inode *inode, struct cifs_sb_info *cifs_sb,
>> +            struct cifsTconInfo *tcon, unsigned int f_flags, __u32 *poplock,
>> +            __u16 *pnetfid, int xid)
>> +{
>> +       int rc;
>> +       int desiredAccess;
>> +       int disposition;
>> +       FILE_ALL_INFO *buf;
>> +
>> +       desiredAccess = cifs_convert_flags(f_flags);
>> +
>> +/*********************************************************************
>> + *  open flag mapping table:
>> + *
>> + *     POSIX Flag            CIFS Disposition
>> + *     ----------            ----------------
>> + *     O_CREAT               FILE_OPEN_IF
>> + *     O_CREAT | O_EXCL      FILE_CREATE
>> + *     O_CREAT | O_TRUNC     FILE_OVERWRITE_IF
>> + *     O_TRUNC               FILE_OVERWRITE
>> + *     none of the above     FILE_OPEN
>> + *
>> + *     Note that there is not a direct match between disposition
>> + *     FILE_SUPERSEDE (ie create whether or not file exists although
>> + *     O_CREAT | O_TRUNC is similar but truncates the existing
>> + *     file rather than creating a new file as FILE_SUPERSEDE does
>> + *     (which uses the attributes / metadata passed in on open call)
>> + *?
>> + *?  O_SYNC is a reasonable match to CIFS writethrough flag
>> + *?  and the read write flags match reasonably.  O_LARGEFILE
>> + *?  is irrelevant because largefile support is always used
>> + *?  by this client. Flags O_APPEND, O_DIRECT, O_DIRECTORY,
>> + *      O_FASYNC, O_NOFOLLOW, O_NONBLOCK need further investigation
>> + *********************************************************************/
>> +
>> +       disposition = cifs_get_disposition(f_flags);
>> +
>> +       /* BB pass O_SYNC flag through on file attributes .. BB */
>> +
>> +       buf = kmalloc(sizeof(FILE_ALL_INFO), GFP_KERNEL);
>> +       if (!buf)
>> +               return -ENOMEM;
>> +
>> +       if (tcon->ses->capabilities & CAP_NT_SMBS)
>> +               rc = CIFSSMBOpen(xid, tcon, full_path, disposition,
>> +                        desiredAccess, CREATE_NOT_DIR, pnetfid, poplock, buf,
>> +                        cifs_sb->local_nls, cifs_sb->mnt_cifs_flags
>> +                                & CIFS_MOUNT_MAP_SPECIAL_CHR);
>> +       else
>> +               rc = SMBLegacyOpen(xid, tcon, full_path, disposition,
>> +                       desiredAccess, CREATE_NOT_DIR, pnetfid, poplock, buf,
>> +                       cifs_sb->local_nls, cifs_sb->mnt_cifs_flags
>> +                               & CIFS_MOUNT_MAP_SPECIAL_CHR);
>> +
>> +       if (rc)
>> +               goto out;
>> +
>> +       if (tcon->unix_ext)
>> +               rc = cifs_get_inode_info_unix(&inode, full_path, inode->i_sb,
>> +                                             xid);
>> +       else
>> +               rc = cifs_get_inode_info(&inode, full_path, buf, inode->i_sb,
>> +                                        xid, pnetfid);
>> +
>
> Just a nit-picking, we probably do not expect a non-posix function
> calling a function for unix!
>

If the server broke posix open but still supports unix extensions we
should use unix specific function, I think.

-- 
Best regards,
Pavel Shilovsky.
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux