| Message ID | 201003241156.42545.piastry@etersoft.ru |
|---|---|
| State | New |
| Headers | show |
On Wed, 24 Mar 2010 11:56:42 +0300 piastry@etersoft.ru wrote: > diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c > index 7cc7f83..b3bbb2b 100644 > --- a/fs/cifs/cifssmb.c > +++ b/fs/cifs/cifssmb.c > @@ -1793,8 +1793,21 @@ CIFSSMBPosixLock(const int xid, struct cifsTconInfo *tcon, > } > parm_data = (struct cifs_posix_lock *) > ((char *)&pSMBr->hdr.Protocol + data_offset); > - if (parm_data->lock_type == cpu_to_le16(CIFS_UNLCK)) > + if (parm_data->lock_type == __constant_cpu_to_le16(CIFS_UNLCK)) > pLockData->fl_type = F_UNLCK; > + else { > + if (parm_data->lock_type == > + __constant_cpu_to_le16(CIFS_RDLCK)) > + pLockData->fl_type = F_RDLCK; > + else if (parm_data->lock_type == > + __constant_cpu_to_le16(CIFS_WRLCK)) > + pLockData->fl_type = F_WRLCK; > + > + pLockData->fl_start = parm_data->start; > + pLockData->fl_end = parm_data->start + > + parm_data->length - 1; > + pLockData->fl_pid = parm_data->pid; > + } > } > > plk_err_exit: > diff --git a/fs/cifs/file.c b/fs/cifs/file.c > index ca2ba7a..d9e8650 100644 > --- a/fs/cifs/file.c > +++ b/fs/cifs/file.c > @@ -838,8 +838,32 @@ int cifs_lock(struct file *file, int cmd, struct file_lock *pfLock) > > } else { > /* if rc == ERR_SHARING_VIOLATION ? */ > - rc = 0; /* do not change lock type to unlock > - since range in use */ > + rc = 0; > + > + if (lockType & LOCKING_ANDX_SHARED_LOCK) { > + pfLock->fl_type = F_WRLCK; > + } else { > + rc = CIFSSMBLock(xid, tcon, netfid, length, > + pfLock->fl_start, 0, 1, > + lockType | LOCKING_ANDX_SHARED_LOCK, > + 0 /* wait flag */); > + if (rc == 0) { > + rc = CIFSSMBLock(xid, tcon, netfid, > + length, pfLock->fl_start, 1, 0, > + lockType | > + LOCKING_ANDX_SHARED_LOCK, > + 0 /* wait flag */); > + pfLock->fl_type = F_RDLCK; > + if (rc != 0) > + cERROR(1, ("Error unlocking " > + "previously locked range %d " > + "during test of lock", rc)); > + rc = 0; > + } else { > + pfLock->fl_type = F_WRLCK; > + rc = 0; > + } > + } > } > > FreeXid(xid); > > Signed-off-by: Pavel Shilovsky <piastryyy@gmail.com> The problem here is that "getlk problem" doesn't really convey exactly what the heck this patch is intended to do. Can you describe what the getlk problem actually is and how this patch fixes it? Thanks,
В сообщении от 24 марта 2010 22:25:59 вы написали: > On Wed, 24 Mar 2010 11:56:42 +0300 > > piastry@etersoft.ru wrote: > > diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c > > index 7cc7f83..b3bbb2b 100644 > > --- a/fs/cifs/cifssmb.c > > +++ b/fs/cifs/cifssmb.c > > @@ -1793,8 +1793,21 @@ CIFSSMBPosixLock(const int xid, struct > > cifsTconInfo *tcon, > > > > } > > parm_data = (struct cifs_posix_lock *) > > > > ((char *)&pSMBr->hdr.Protocol + data_offset); > > > > - if (parm_data->lock_type == cpu_to_le16(CIFS_UNLCK)) > > + if (parm_data->lock_type == __constant_cpu_to_le16(CIFS_UNLCK)) > > > > pLockData->fl_type = F_UNLCK; > > > > + else { > > + if (parm_data->lock_type == > > + __constant_cpu_to_le16(CIFS_RDLCK)) > > + pLockData->fl_type = F_RDLCK; > > + else if (parm_data->lock_type == > > + __constant_cpu_to_le16(CIFS_WRLCK)) > > + pLockData->fl_type = F_WRLCK; > > + > > + pLockData->fl_start = parm_data->start; > > + pLockData->fl_end = parm_data->start + > > + parm_data->length - 1; > > + pLockData->fl_pid = parm_data->pid; > > + } > > > > } > > > > plk_err_exit: > > diff --git a/fs/cifs/file.c b/fs/cifs/file.c > > index ca2ba7a..d9e8650 100644 > > --- a/fs/cifs/file.c > > +++ b/fs/cifs/file.c > > @@ -838,8 +838,32 @@ int cifs_lock(struct file *file, int cmd, struct > > file_lock *pfLock) > > > > } else { > > > > /* if rc == ERR_SHARING_VIOLATION ? */ > > > > - rc = 0; /* do not change lock type to unlock > > - since range in use */ > > + rc = 0; > > + > > + if (lockType & LOCKING_ANDX_SHARED_LOCK) { > > + pfLock->fl_type = F_WRLCK; > > + } else { > > + rc = CIFSSMBLock(xid, tcon, netfid, length, > > + pfLock->fl_start, 0, 1, > > + lockType | LOCKING_ANDX_SHARED_LOCK, > > + 0 /* wait flag */); > > + if (rc == 0) { > > + rc = CIFSSMBLock(xid, tcon, netfid, > > + length, pfLock->fl_start, 1, 0, > > + lockType | > > + LOCKING_ANDX_SHARED_LOCK, > > + 0 /* wait flag */); > > + pfLock->fl_type = F_RDLCK; > > + if (rc != 0) > > + cERROR(1, ("Error unlocking " > > + "previously locked range %d " > > + "during test of lock", rc)); > > + rc = 0; > > + } else { > > + pfLock->fl_type = F_WRLCK; > > + rc = 0; > > + } > > + } > > > > } > > > > FreeXid(xid); > > > > Signed-off-by: Pavel Shilovsky <piastryyy@gmail.com> > > The problem here is that "getlk problem" doesn't really convey exactly > what the heck this patch is intended to do. Can you describe what the > getlk problem actually is and how this patch fixes it? > > Thanks, СIFS client doesn't everwrite flock structure during GET_LK request without this patch. According to fcntl man page it isn't right. If we have preventing lock, cifs should overwrite flock structure with info about preventing lock. If we haven't preventing lock, cifs should leave it unchanged except for the lock type (change it to F_UNLCK). My patch does exactly what we need. As for Unix Extension locking style I think it's clear enough. As for Windows locking style it is more complicated. We can't recognize an offset, length and pid of preventing lock due to CIFS specification with normal way but we can calculate the type. The algorithm follows. 1. We had GET_LK with SHARED_LOCK - tried to set it and failed - it means that we have EXCLUSIVE_LOCK, that prevent us. goto exit. 2. We had GET_LK with EXCLUSIVE_LOCK - tried set it and failed - it means that we have preventing lock but don't know what kind it is. goto step 3. 3. We tried to set it again but with SHARED_LOCK: 3.1 If SET_LK failed - it means that we have EXCLUSIVE_LOCK, that prevent us. goto exit. 3.2 If SET_LK didn't failed - it means that we have SHARED_LOCK, that prevent us. goto exit. -- Best regards, Pavel Shilovsky.
On Wed, 24 Mar 2010 11:56:42 +0300 piastry@etersoft.ru wrote: > diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c > index 7cc7f83..b3bbb2b 100644 > --- a/fs/cifs/cifssmb.c > +++ b/fs/cifs/cifssmb.c > @@ -1793,8 +1793,21 @@ CIFSSMBPosixLock(const int xid, struct cifsTconInfo *tcon, > } > parm_data = (struct cifs_posix_lock *) > ((char *)&pSMBr->hdr.Protocol + data_offset); > - if (parm_data->lock_type == cpu_to_le16(CIFS_UNLCK)) > + if (parm_data->lock_type == __constant_cpu_to_le16(CIFS_UNLCK)) > pLockData->fl_type = F_UNLCK; > + else { > + if (parm_data->lock_type == > + __constant_cpu_to_le16(CIFS_RDLCK)) > + pLockData->fl_type = F_RDLCK; > + else if (parm_data->lock_type == > + __constant_cpu_to_le16(CIFS_WRLCK)) > + pLockData->fl_type = F_WRLCK; > + > + pLockData->fl_start = parm_data->start; > + pLockData->fl_end = parm_data->start + > + parm_data->length - 1; > + pLockData->fl_pid = parm_data->pid; > + } > } > > plk_err_exit: > diff --git a/fs/cifs/file.c b/fs/cifs/file.c > index ca2ba7a..d9e8650 100644 > --- a/fs/cifs/file.c > +++ b/fs/cifs/file.c > @@ -838,8 +838,32 @@ int cifs_lock(struct file *file, int cmd, struct file_lock *pfLock) > > } else { > /* if rc == ERR_SHARING_VIOLATION ? */ > - rc = 0; /* do not change lock type to unlock > - since range in use */ > + rc = 0; > + > + if (lockType & LOCKING_ANDX_SHARED_LOCK) { > + pfLock->fl_type = F_WRLCK; > + } else { > + rc = CIFSSMBLock(xid, tcon, netfid, length, > + pfLock->fl_start, 0, 1, > + lockType | LOCKING_ANDX_SHARED_LOCK, > + 0 /* wait flag */); > + if (rc == 0) { > + rc = CIFSSMBLock(xid, tcon, netfid, > + length, pfLock->fl_start, 1, 0, > + lockType | > + LOCKING_ANDX_SHARED_LOCK, > + 0 /* wait flag */); > + pfLock->fl_type = F_RDLCK; > + if (rc != 0) > + cERROR(1, ("Error unlocking " > + "previously locked range %d " > + "during test of lock", rc)); > + rc = 0; > + } else { > + pfLock->fl_type = F_WRLCK; > + rc = 0; > + } > + } > } > > FreeXid(xid); > > Signed-off-by: Pavel Shilovsky <piastryyy@gmail.com> I think this looks reasonable, however it would be preferable to have a proper description and SoB line in the right place. That makes it easier to pull into the tree. In the future consider using git-format-patch to generate them. Reviewed-by: Jeff Layton <jlayton@samba.org>
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c index 7cc7f83..b3bbb2b 100644 --- a/fs/cifs/cifssmb.c +++ b/fs/cifs/cifssmb.c @@ -1793,8 +1793,21 @@ CIFSSMBPosixLock(const int xid, struct cifsTconInfo *tcon, } parm_data = (struct cifs_posix_lock *) ((char *)&pSMBr->hdr.Protocol + data_offset); - if (parm_data->lock_type == cpu_to_le16(CIFS_UNLCK)) + if (parm_data->lock_type == __constant_cpu_to_le16(CIFS_UNLCK)) pLockData->fl_type = F_UNLCK; + else { + if (parm_data->lock_type == + __constant_cpu_to_le16(CIFS_RDLCK)) + pLockData->fl_type = F_RDLCK; + else if (parm_data->lock_type == + __constant_cpu_to_le16(CIFS_WRLCK)) + pLockData->fl_type = F_WRLCK; + + pLockData->fl_start = parm_data->start; + pLockData->fl_end = parm_data->start + + parm_data->length - 1; + pLockData->fl_pid = parm_data->pid; + } } plk_err_exit: diff --git a/fs/cifs/file.c b/fs/cifs/file.c index ca2ba7a..d9e8650 100644 --- a/fs/cifs/file.c +++ b/fs/cifs/file.c @@ -838,8 +838,32 @@ int cifs_lock(struct file *file, int cmd, struct file_lock *pfLock) } else { /* if rc == ERR_SHARING_VIOLATION ? */ - rc = 0; /* do not change lock type to unlock - since range in use */ + rc = 0; + + if (lockType & LOCKING_ANDX_SHARED_LOCK) { + pfLock->fl_type = F_WRLCK; + } else { + rc = CIFSSMBLock(xid, tcon, netfid, length, + pfLock->fl_start, 0, 1, + lockType | LOCKING_ANDX_SHARED_LOCK, + 0 /* wait flag */); + if (rc == 0) { + rc = CIFSSMBLock(xid, tcon, netfid, + length, pfLock->fl_start, 1, 0, + lockType | + LOCKING_ANDX_SHARED_LOCK, + 0 /* wait flag */); + pfLock->fl_type = F_RDLCK; + if (rc != 0) + cERROR(1, ("Error unlocking " + "previously locked range %d " + "during test of lock", rc)); + rc = 0; + } else { + pfLock->fl_type = F_WRLCK; + rc = 0; + } + } } FreeXid(xid);
Signed-off-by: Pavel Shilovsky <piastryyy@gmail.com>