Skip to content

Commit ec637e3

Browse files
author
Steve French
committed
[CIFS] Avoid extra large buffer allocation (and memcpy) in cifs_readpages
Signed-off-by: Steve French <sfrench@us.ibm.com>
1 parent c89a86b commit ec637e3

11 files changed

+156
-108
lines changed

fs/cifs/cifs_debug.c

+19-20
Original file line numberDiff line numberDiff line change
@@ -401,8 +401,8 @@ static read_proc_t ntlmv2_enabled_read;
401401
static write_proc_t ntlmv2_enabled_write;
402402
static read_proc_t packet_signing_enabled_read;
403403
static write_proc_t packet_signing_enabled_write;
404-
static read_proc_t quotaEnabled_read;
405-
static write_proc_t quotaEnabled_write;
404+
static read_proc_t experimEnabled_read;
405+
static write_proc_t experimEnabled_write;
406406
static read_proc_t linuxExtensionsEnabled_read;
407407
static write_proc_t linuxExtensionsEnabled_write;
408408

@@ -442,9 +442,9 @@ cifs_proc_init(void)
442442
pde->write_proc = oplockEnabled_write;
443443

444444
pde = create_proc_read_entry("Experimental", 0, proc_fs_cifs,
445-
quotaEnabled_read, NULL);
445+
experimEnabled_read, NULL);
446446
if (pde)
447-
pde->write_proc = quotaEnabled_write;
447+
pde->write_proc = experimEnabled_write;
448448

449449
pde = create_proc_read_entry("LinuxExtensionsEnabled", 0, proc_fs_cifs,
450450
linuxExtensionsEnabled_read, NULL);
@@ -586,14 +586,13 @@ oplockEnabled_write(struct file *file, const char __user *buffer,
586586
}
587587

588588
static int
589-
quotaEnabled_read(char *page, char **start, off_t off,
589+
experimEnabled_read(char *page, char **start, off_t off,
590590
int count, int *eof, void *data)
591591
{
592592
int len;
593593

594594
len = sprintf(page, "%d\n", experimEnabled);
595-
/* could also check if quotas are enabled in kernel
596-
as a whole first */
595+
597596
len -= off;
598597
*start = page + off;
599598

@@ -608,21 +607,23 @@ quotaEnabled_read(char *page, char **start, off_t off,
608607
return len;
609608
}
610609
static int
611-
quotaEnabled_write(struct file *file, const char __user *buffer,
610+
experimEnabled_write(struct file *file, const char __user *buffer,
612611
unsigned long count, void *data)
613612
{
614-
char c;
615-
int rc;
613+
char c;
614+
int rc;
616615

617-
rc = get_user(c, buffer);
618-
if (rc)
619-
return rc;
620-
if (c == '0' || c == 'n' || c == 'N')
621-
experimEnabled = 0;
622-
else if (c == '1' || c == 'y' || c == 'Y')
623-
experimEnabled = 1;
616+
rc = get_user(c, buffer);
617+
if (rc)
618+
return rc;
619+
if (c == '0' || c == 'n' || c == 'N')
620+
experimEnabled = 0;
621+
else if (c == '1' || c == 'y' || c == 'Y')
622+
experimEnabled = 1;
623+
else if (c == '2')
624+
experimEnabled = 2;
624625

625-
return count;
626+
return count;
626627
}
627628

628629
static int
@@ -632,8 +633,6 @@ linuxExtensionsEnabled_read(char *page, char **start, off_t off,
632633
int len;
633634

634635
len = sprintf(page, "%d\n", linuxExtEnabled);
635-
/* could also check if quotas are enabled in kernel
636-
as a whole first */
637636
len -= off;
638637
*start = page + off;
639638

fs/cifs/cifsfs.c

+3-2
Original file line numberDiff line numberDiff line change
@@ -733,7 +733,7 @@ cifs_init_request_bufs(void)
733733
kmem_cache_destroy(cifs_req_cachep);
734734
return -ENOMEM;
735735
}
736-
/* 256 (MAX_CIFS_HDR_SIZE bytes is enough for most SMB responses and
736+
/* MAX_CIFS_SMALL_BUFFER_SIZE bytes is enough for most SMB responses and
737737
almost all handle based requests (but not write response, nor is it
738738
sufficient for path based requests). A smaller size would have
739739
been more efficient (compacting multiple slab items on one 4k page)
@@ -742,7 +742,8 @@ cifs_init_request_bufs(void)
742742
efficient to alloc 1 per page off the slab compared to 17K (5page)
743743
alloc of large cifs buffers even when page debugging is on */
744744
cifs_sm_req_cachep = kmem_cache_create("cifs_small_rq",
745-
MAX_CIFS_HDR_SIZE, 0, SLAB_HWCACHE_ALIGN, NULL, NULL);
745+
MAX_CIFS_SMALL_BUFFER_SIZE, 0, SLAB_HWCACHE_ALIGN,
746+
NULL, NULL);
746747
if (cifs_sm_req_cachep == NULL) {
747748
mempool_destroy(cifs_req_poolp);
748749
kmem_cache_destroy(cifs_req_cachep);

fs/cifs/cifsglob.h

+7-1
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,7 @@ struct cifs_search_info {
285285
unsigned endOfSearch:1;
286286
unsigned emptyDir:1;
287287
unsigned unicode:1;
288+
unsigned smallBuf:1; /* so we know which buf_release function to call */
288289
};
289290

290291
struct cifsFileInfo {
@@ -420,7 +421,12 @@ struct dir_notify_req {
420421
#define MID_RESPONSE_RECEIVED 4
421422
#define MID_RETRY_NEEDED 8 /* session closed while this request out */
422423
#define MID_NO_RESP_NEEDED 0x10
423-
#define MID_SMALL_BUFFER 0x20 /* 112 byte response buffer instead of 4K */
424+
425+
/* Types of response buffer returned from SendReceive2 */
426+
#define CIFS_NO_BUFFER 0 /* Response buffer not returned */
427+
#define CIFS_SMALL_BUFFER 1
428+
#define CIFS_LARGE_BUFFER 2
429+
#define CIFS_IOVEC 4 /* array of response buffers */
424430

425431
/*
426432
*****************************************************************

fs/cifs/cifspdu.h

+5-1
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,11 @@
8080
#define NT_TRANSACT_GET_USER_QUOTA 0x07
8181
#define NT_TRANSACT_SET_USER_QUOTA 0x08
8282

83-
#define MAX_CIFS_HDR_SIZE 256 /* is future chained NTCreateXReadX bigger? */
83+
#define MAX_CIFS_SMALL_BUFFER_SIZE 448 /* big enough for most */
84+
/* future chained NTCreateXReadX bigger, but for time being NTCreateX biggest */
85+
/* among the requests (NTCreateX response is bigger with wct of 34) */
86+
#define MAX_CIFS_HDR_SIZE 0x58 /* 4 len + 32 hdr + (2*24 wct) + 2 bct + 2 pad */
87+
#define CIFS_SMALL_PATH 120 /* allows for (448-88)/3 */
8488

8589
/* internal cifs vfs structures */
8690
/*****************************************************************

fs/cifs/cifsproto.h

+8-6
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ extern int SendReceive(const unsigned int /* xid */ , struct cifsSesInfo *,
4949
int * /* bytes returned */ , const int long_op);
5050
extern int SendReceive2(const unsigned int /* xid */ , struct cifsSesInfo *,
5151
struct kvec *, int /* nvec to send */,
52-
int * /* bytes returned */ , const int long_op);
52+
int * /* type of buf returned */ , const int long_op);
5353
extern int checkSMBhdr(struct smb_hdr *smb, __u16 mid);
5454
extern int checkSMB(struct smb_hdr *smb, __u16 mid, int length);
5555
extern int is_valid_oplock_break(struct smb_hdr *smb);
@@ -93,11 +93,12 @@ extern int CIFSTCon(unsigned int xid, struct cifsSesInfo *ses,
9393
const struct nls_table *);
9494

9595
extern int CIFSFindFirst(const int xid, struct cifsTconInfo *tcon,
96-
const char *searchName, const struct nls_table *nls_codepage,
97-
__u16 *searchHandle, struct cifs_search_info * psrch_inf, int map, const char dirsep);
96+
const char *searchName, const struct nls_table *nls_codepage,
97+
__u16 *searchHandle, struct cifs_search_info * psrch_inf,
98+
int map, const char dirsep);
9899

99100
extern int CIFSFindNext(const int xid, struct cifsTconInfo *tcon,
100-
__u16 searchHandle, struct cifs_search_info * psrch_inf);
101+
__u16 searchHandle, struct cifs_search_info * psrch_inf);
101102

102103
extern int CIFSFindClose(const int, struct cifsTconInfo *tcon,
103104
const __u16 search_handle);
@@ -230,8 +231,9 @@ extern int CIFSSMBClose(const int xid, struct cifsTconInfo *tcon,
230231
const int smb_file_id);
231232

232233
extern int CIFSSMBRead(const int xid, struct cifsTconInfo *tcon,
233-
const int netfid, unsigned int count,
234-
const __u64 lseek, unsigned int *nbytes, char **buf);
234+
const int netfid, unsigned int count,
235+
const __u64 lseek, unsigned int *nbytes, char **buf,
236+
int * return_buf_type);
235237
extern int CIFSSMBWrite(const int xid, struct cifsTconInfo *tcon,
236238
const int netfid, const unsigned int count,
237239
const __u64 lseek, unsigned int *nbytes,

fs/cifs/cifssmb.c

+58-35
Original file line numberDiff line numberDiff line change
@@ -958,21 +958,19 @@ CIFSSMBOpen(const int xid, struct cifsTconInfo *tcon,
958958
return rc;
959959
}
960960

961-
/* If no buffer passed in, then caller wants to do the copy
962-
as in the case of readpages so the SMB buffer must be
963-
freed by the caller */
964-
965961
int
966962
CIFSSMBRead(const int xid, struct cifsTconInfo *tcon,
967-
const int netfid, const unsigned int count,
968-
const __u64 lseek, unsigned int *nbytes, char **buf)
963+
const int netfid, const unsigned int count,
964+
const __u64 lseek, unsigned int *nbytes, char **buf,
965+
int * pbuf_type)
969966
{
970967
int rc = -EACCES;
971968
READ_REQ *pSMB = NULL;
972969
READ_RSP *pSMBr = NULL;
973970
char *pReadData = NULL;
974-
int bytes_returned;
975971
int wct;
972+
int resp_buf_type = 0;
973+
struct kvec iov[1];
976974

977975
cFYI(1,("Reading %d bytes on fid %d",count,netfid));
978976
if(tcon->ses->capabilities & CAP_LARGE_FILES)
@@ -981,22 +979,21 @@ CIFSSMBRead(const int xid, struct cifsTconInfo *tcon,
981979
wct = 10; /* old style read */
982980

983981
*nbytes = 0;
984-
rc = smb_init(SMB_COM_READ_ANDX, wct, tcon, (void **) &pSMB,
985-
(void **) &pSMBr);
982+
rc = small_smb_init(SMB_COM_READ_ANDX, wct, tcon, (void **) &pSMB);
986983
if (rc)
987984
return rc;
988985

989986
/* tcon and ses pointer are checked in smb_init */
990987
if (tcon->ses->server == NULL)
991988
return -ECONNABORTED;
992989

993-
pSMB->AndXCommand = 0xFF; /* none */
990+
pSMB->AndXCommand = 0xFF; /* none */
994991
pSMB->Fid = netfid;
995992
pSMB->OffsetLow = cpu_to_le32(lseek & 0xFFFFFFFF);
996993
if(wct == 12)
997994
pSMB->OffsetHigh = cpu_to_le32(lseek >> 32);
998-
else if((lseek >> 32) > 0) /* can not handle this big offset for old */
999-
return -EIO;
995+
else if((lseek >> 32) > 0) /* can not handle this big offset for old */
996+
return -EIO;
1000997

1001998
pSMB->Remaining = 0;
1002999
pSMB->MaxCount = cpu_to_le16(count & 0xFFFF);
@@ -1005,14 +1002,18 @@ CIFSSMBRead(const int xid, struct cifsTconInfo *tcon,
10051002
pSMB->ByteCount = 0; /* no need to do le conversion since 0 */
10061003
else {
10071004
/* old style read */
1008-
struct smb_com_readx_req * pSMBW =
1005+
struct smb_com_readx_req * pSMBW =
10091006
(struct smb_com_readx_req *)pSMB;
1010-
pSMBW->ByteCount = 0;
1007+
pSMBW->ByteCount = 0;
10111008
}
1012-
1013-
rc = SendReceive(xid, tcon->ses, (struct smb_hdr *) pSMB,
1014-
(struct smb_hdr *) pSMBr, &bytes_returned, 0);
1009+
1010+
iov[0].iov_base = (char *)pSMB;
1011+
iov[0].iov_len = pSMB->hdr.smb_buf_length + 4;
1012+
rc = SendReceive2(xid, tcon->ses, iov,
1013+
1 /* num iovecs */,
1014+
&resp_buf_type, 0);
10151015
cifs_stats_inc(&tcon->num_reads);
1016+
pSMBr = (READ_RSP *)iov[0].iov_base;
10161017
if (rc) {
10171018
cERROR(1, ("Send error in read = %d", rc));
10181019
} else {
@@ -1022,33 +1023,43 @@ CIFSSMBRead(const int xid, struct cifsTconInfo *tcon,
10221023
*nbytes = data_length;
10231024

10241025
/*check that DataLength would not go beyond end of SMB */
1025-
if ((data_length > CIFSMaxBufSize)
1026+
if ((data_length > CIFSMaxBufSize)
10261027
|| (data_length > count)) {
10271028
cFYI(1,("bad length %d for count %d",data_length,count));
10281029
rc = -EIO;
10291030
*nbytes = 0;
10301031
} else {
1031-
pReadData =
1032-
(char *) (&pSMBr->hdr.Protocol) +
1032+
pReadData = (char *) (&pSMBr->hdr.Protocol) +
10331033
le16_to_cpu(pSMBr->DataOffset);
1034-
/* if(rc = copy_to_user(buf, pReadData, data_length)) {
1035-
cERROR(1,("Faulting on read rc = %d",rc));
1036-
rc = -EFAULT;
1037-
}*/ /* can not use copy_to_user when using page cache*/
1034+
/* if(rc = copy_to_user(buf, pReadData, data_length)) {
1035+
cERROR(1,("Faulting on read rc = %d",rc));
1036+
rc = -EFAULT;
1037+
}*/ /* can not use copy_to_user when using page cache*/
10381038
if(*buf)
1039-
memcpy(*buf,pReadData,data_length);
1039+
memcpy(*buf,pReadData,data_length);
10401040
}
10411041
}
1042-
if(*buf)
1043-
cifs_buf_release(pSMB);
1044-
else
1045-
*buf = (char *)pSMB;
10461042

1047-
/* Note: On -EAGAIN error only caller can retry on handle based calls
1043+
cifs_small_buf_release(pSMB);
1044+
if(*buf) {
1045+
if(resp_buf_type == CIFS_SMALL_BUFFER)
1046+
cifs_small_buf_release(iov[0].iov_base);
1047+
else if(resp_buf_type == CIFS_LARGE_BUFFER)
1048+
cifs_buf_release(iov[0].iov_base);
1049+
} else /* return buffer to caller to free */ /* BB FIXME how do we tell caller if it is not a large buffer */ {
1050+
*buf = iov[0].iov_base;
1051+
if(resp_buf_type == CIFS_SMALL_BUFFER)
1052+
*pbuf_type = CIFS_SMALL_BUFFER;
1053+
else if(resp_buf_type == CIFS_LARGE_BUFFER)
1054+
*pbuf_type = CIFS_LARGE_BUFFER;
1055+
}
1056+
1057+
/* Note: On -EAGAIN error only caller can retry on handle based calls
10481058
since file handle passed in no longer valid */
10491059
return rc;
10501060
}
10511061

1062+
10521063
int
10531064
CIFSSMBWrite(const int xid, struct cifsTconInfo *tcon,
10541065
const int netfid, const unsigned int count,
@@ -1163,10 +1174,10 @@ CIFSSMBWrite2(const int xid, struct cifsTconInfo *tcon,
11631174
{
11641175
int rc = -EACCES;
11651176
WRITE_REQ *pSMB = NULL;
1166-
int bytes_returned, wct;
1177+
int wct;
11671178
int smb_hdr_len;
1179+
int resp_buf_type = 0;
11681180

1169-
/* BB removeme BB */
11701181
cFYI(1,("write2 at %lld %d bytes", (long long)offset, count));
11711182

11721183
if(tcon->ses->capabilities & CAP_LARGE_FILES)
@@ -1209,22 +1220,34 @@ CIFSSMBWrite2(const int xid, struct cifsTconInfo *tcon,
12091220
pSMBW->ByteCount = cpu_to_le16(count + 5);
12101221
}
12111222
iov[0].iov_base = pSMB;
1212-
iov[0].iov_len = smb_hdr_len + 4;
1223+
if(wct == 14)
1224+
iov[0].iov_len = smb_hdr_len + 4;
1225+
else /* wct == 12 pad bigger by four bytes */
1226+
iov[0].iov_len = smb_hdr_len + 8;
1227+
12131228

1214-
rc = SendReceive2(xid, tcon->ses, iov, n_vec + 1, &bytes_returned,
1229+
rc = SendReceive2(xid, tcon->ses, iov, n_vec + 1, &resp_buf_type,
12151230
long_op);
12161231
cifs_stats_inc(&tcon->num_writes);
12171232
if (rc) {
12181233
cFYI(1, ("Send error Write2 = %d", rc));
12191234
*nbytes = 0;
1235+
} else if(resp_buf_type == 0) {
1236+
/* presumably this can not happen, but best to be safe */
1237+
rc = -EIO;
1238+
*nbytes = 0;
12201239
} else {
1221-
WRITE_RSP * pSMBr = (WRITE_RSP *)pSMB;
1240+
WRITE_RSP * pSMBr = (WRITE_RSP *)iov[0].iov_base;
12221241
*nbytes = le16_to_cpu(pSMBr->CountHigh);
12231242
*nbytes = (*nbytes) << 16;
12241243
*nbytes += le16_to_cpu(pSMBr->Count);
12251244
}
12261245

12271246
cifs_small_buf_release(pSMB);
1247+
if(resp_buf_type == CIFS_SMALL_BUFFER)
1248+
cifs_small_buf_release(iov[0].iov_base);
1249+
else if(resp_buf_type == CIFS_LARGE_BUFFER)
1250+
cifs_buf_release(iov[0].iov_base);
12281251

12291252
/* Note: On -EAGAIN error only caller can retry on handle based calls
12301253
since file handle passed in no longer valid */

fs/cifs/connect.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -514,7 +514,7 @@ cifs_demultiplex_thread(struct TCP_Server_Info *server)
514514
/* else length ok */
515515
reconnect = 0;
516516

517-
if(pdu_length > MAX_CIFS_HDR_SIZE - 4) {
517+
if(pdu_length > MAX_CIFS_SMALL_BUFFER_SIZE - 4) {
518518
isLargeBuf = TRUE;
519519
memcpy(bigbuf, smallbuf, 4);
520520
smb_buffer = bigbuf;

0 commit comments

Comments
 (0)