Skip to content

Commit b64af6b

Browse files
pcacjrSteve French
authored andcommitted
smb: client: fix perf regression with deferred closes
Customer reported that one of their applications started failing to open files with STATUS_INSUFFICIENT_RESOURCES due to NetApp server hitting the maximum number of opens to same file that it would allow for a single client connection. It turned out the client was failing to reuse open handles with deferred closes because matching ->f_flags directly without masking off O_CREAT|O_EXCL|O_TRUNC bits first broke the comparision and then client ended up with thousands of deferred closes to same file. Those bits are already satisfied on the original open, so no need to check them against existing open handles. Reproducer: #include <stdio.h> #include <stdlib.h> #include <string.h> #include <unistd.h> #include <fcntl.h> #include <pthread.h> #define NR_THREADS 4 #define NR_ITERATIONS 2500 #define TEST_FILE "/mnt/1/test/dir/foo" static char buf[64]; static void *worker(void *arg) { int i, j; int fd; for (i = 0; i < NR_ITERATIONS; i++) { fd = open(TEST_FILE, O_WRONLY|O_CREAT|O_APPEND, 0666); for (j = 0; j < 16; j++) write(fd, buf, sizeof(buf)); close(fd); } } int main(int argc, char *argv[]) { pthread_t t[NR_THREADS]; int fd; int i; fd = open(TEST_FILE, O_WRONLY|O_CREAT|O_TRUNC, 0666); close(fd); memset(buf, 'a', sizeof(buf)); for (i = 0; i < NR_THREADS; i++) pthread_create(&t[i], NULL, worker, NULL); for (i = 0; i < NR_THREADS; i++) pthread_join(t[i], NULL); return 0; } Before patch: $ mount.cifs //srv/share /mnt/1 -o ... $ mkdir -p /mnt/1/test/dir $ gcc repro.c && ./a.out ... number of opens: 1391 After patch: $ mount.cifs //srv/share /mnt/1 -o ... $ mkdir -p /mnt/1/test/dir $ gcc repro.c && ./a.out ... number of opens: 1 Cc: [email protected] Cc: David Howells <[email protected]> Cc: Jay Shin <[email protected]> Cc: Pierguido Lambri <[email protected]> Fixes: b8ea3b1 ("smb: enable reuse of deferred file handles for write operations") Acked-by: Shyam Prasad N <[email protected]> Signed-off-by: Paulo Alcantara (Red Hat) <[email protected]> Signed-off-by: Steve French <[email protected]>
1 parent 9331005 commit b64af6b

File tree

1 file changed

+6
-3
lines changed

1 file changed

+6
-3
lines changed

fs/smb/client/file.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -999,15 +999,18 @@ int cifs_open(struct inode *inode, struct file *file)
999999
rc = cifs_get_readable_path(tcon, full_path, &cfile);
10001000
}
10011001
if (rc == 0) {
1002-
if (file->f_flags == cfile->f_flags) {
1002+
unsigned int oflags = file->f_flags & ~(O_CREAT|O_EXCL|O_TRUNC);
1003+
unsigned int cflags = cfile->f_flags & ~(O_CREAT|O_EXCL|O_TRUNC);
1004+
1005+
if (cifs_convert_flags(oflags, 0) == cifs_convert_flags(cflags, 0) &&
1006+
(oflags & (O_SYNC|O_DIRECT)) == (cflags & (O_SYNC|O_DIRECT))) {
10031007
file->private_data = cfile;
10041008
spin_lock(&CIFS_I(inode)->deferred_lock);
10051009
cifs_del_deferred_close(cfile);
10061010
spin_unlock(&CIFS_I(inode)->deferred_lock);
10071011
goto use_cache;
1008-
} else {
1009-
_cifsFileInfo_put(cfile, true, false);
10101012
}
1013+
_cifsFileInfo_put(cfile, true, false);
10111014
} else {
10121015
/* hard link on the defeered close file */
10131016
rc = cifs_get_hardlink_path(tcon, inode, file);

0 commit comments

Comments
 (0)