Skip to content

Commit

Permalink
[CIFS] Avoid extra large buffer allocation (and memcpy) in cifs_readp…
Browse files Browse the repository at this point in the history
…ages

Signed-off-by: Steve French <[email protected]>
  • Loading branch information
Steve French committed Dec 13, 2005
1 parent c89a86b commit ec637e3
Show file tree
Hide file tree
Showing 11 changed files with 156 additions and 108 deletions.
39 changes: 19 additions & 20 deletions fs/cifs/cifs_debug.c
Original file line number Diff line number Diff line change
Expand Up @@ -401,8 +401,8 @@ static read_proc_t ntlmv2_enabled_read;
static write_proc_t ntlmv2_enabled_write;
static read_proc_t packet_signing_enabled_read;
static write_proc_t packet_signing_enabled_write;
static read_proc_t quotaEnabled_read;
static write_proc_t quotaEnabled_write;
static read_proc_t experimEnabled_read;
static write_proc_t experimEnabled_write;
static read_proc_t linuxExtensionsEnabled_read;
static write_proc_t linuxExtensionsEnabled_write;

Expand Down Expand Up @@ -442,9 +442,9 @@ cifs_proc_init(void)
pde->write_proc = oplockEnabled_write;

pde = create_proc_read_entry("Experimental", 0, proc_fs_cifs,
quotaEnabled_read, NULL);
experimEnabled_read, NULL);
if (pde)
pde->write_proc = quotaEnabled_write;
pde->write_proc = experimEnabled_write;

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

static int
quotaEnabled_read(char *page, char **start, off_t off,
experimEnabled_read(char *page, char **start, off_t off,
int count, int *eof, void *data)
{
int len;

len = sprintf(page, "%d\n", experimEnabled);
/* could also check if quotas are enabled in kernel
as a whole first */

len -= off;
*start = page + off;

Expand All @@ -608,21 +607,23 @@ quotaEnabled_read(char *page, char **start, off_t off,
return len;
}
static int
quotaEnabled_write(struct file *file, const char __user *buffer,
experimEnabled_write(struct file *file, const char __user *buffer,
unsigned long count, void *data)
{
char c;
int rc;
char c;
int rc;

rc = get_user(c, buffer);
if (rc)
return rc;
if (c == '0' || c == 'n' || c == 'N')
experimEnabled = 0;
else if (c == '1' || c == 'y' || c == 'Y')
experimEnabled = 1;
rc = get_user(c, buffer);
if (rc)
return rc;
if (c == '0' || c == 'n' || c == 'N')
experimEnabled = 0;
else if (c == '1' || c == 'y' || c == 'Y')
experimEnabled = 1;
else if (c == '2')
experimEnabled = 2;

return count;
return count;
}

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

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

Expand Down
5 changes: 3 additions & 2 deletions fs/cifs/cifsfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -733,7 +733,7 @@ cifs_init_request_bufs(void)
kmem_cache_destroy(cifs_req_cachep);
return -ENOMEM;
}
/* 256 (MAX_CIFS_HDR_SIZE bytes is enough for most SMB responses and
/* MAX_CIFS_SMALL_BUFFER_SIZE bytes is enough for most SMB responses and
almost all handle based requests (but not write response, nor is it
sufficient for path based requests). A smaller size would have
been more efficient (compacting multiple slab items on one 4k page)
Expand All @@ -742,7 +742,8 @@ cifs_init_request_bufs(void)
efficient to alloc 1 per page off the slab compared to 17K (5page)
alloc of large cifs buffers even when page debugging is on */
cifs_sm_req_cachep = kmem_cache_create("cifs_small_rq",
MAX_CIFS_HDR_SIZE, 0, SLAB_HWCACHE_ALIGN, NULL, NULL);
MAX_CIFS_SMALL_BUFFER_SIZE, 0, SLAB_HWCACHE_ALIGN,
NULL, NULL);
if (cifs_sm_req_cachep == NULL) {
mempool_destroy(cifs_req_poolp);
kmem_cache_destroy(cifs_req_cachep);
Expand Down
8 changes: 7 additions & 1 deletion fs/cifs/cifsglob.h
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,7 @@ struct cifs_search_info {
unsigned endOfSearch:1;
unsigned emptyDir:1;
unsigned unicode:1;
unsigned smallBuf:1; /* so we know which buf_release function to call */
};

struct cifsFileInfo {
Expand Down Expand Up @@ -420,7 +421,12 @@ struct dir_notify_req {
#define MID_RESPONSE_RECEIVED 4
#define MID_RETRY_NEEDED 8 /* session closed while this request out */
#define MID_NO_RESP_NEEDED 0x10
#define MID_SMALL_BUFFER 0x20 /* 112 byte response buffer instead of 4K */

/* Types of response buffer returned from SendReceive2 */
#define CIFS_NO_BUFFER 0 /* Response buffer not returned */
#define CIFS_SMALL_BUFFER 1
#define CIFS_LARGE_BUFFER 2
#define CIFS_IOVEC 4 /* array of response buffers */

/*
*****************************************************************
Expand Down
6 changes: 5 additions & 1 deletion fs/cifs/cifspdu.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,11 @@
#define NT_TRANSACT_GET_USER_QUOTA 0x07
#define NT_TRANSACT_SET_USER_QUOTA 0x08

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

/* internal cifs vfs structures */
/*****************************************************************
Expand Down
14 changes: 8 additions & 6 deletions fs/cifs/cifsproto.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ extern int SendReceive(const unsigned int /* xid */ , struct cifsSesInfo *,
int * /* bytes returned */ , const int long_op);
extern int SendReceive2(const unsigned int /* xid */ , struct cifsSesInfo *,
struct kvec *, int /* nvec to send */,
int * /* bytes returned */ , const int long_op);
int * /* type of buf returned */ , const int long_op);
extern int checkSMBhdr(struct smb_hdr *smb, __u16 mid);
extern int checkSMB(struct smb_hdr *smb, __u16 mid, int length);
extern int is_valid_oplock_break(struct smb_hdr *smb);
Expand Down Expand Up @@ -93,11 +93,12 @@ extern int CIFSTCon(unsigned int xid, struct cifsSesInfo *ses,
const struct nls_table *);

extern int CIFSFindFirst(const int xid, struct cifsTconInfo *tcon,
const char *searchName, const struct nls_table *nls_codepage,
__u16 *searchHandle, struct cifs_search_info * psrch_inf, int map, const char dirsep);
const char *searchName, const struct nls_table *nls_codepage,
__u16 *searchHandle, struct cifs_search_info * psrch_inf,
int map, const char dirsep);

extern int CIFSFindNext(const int xid, struct cifsTconInfo *tcon,
__u16 searchHandle, struct cifs_search_info * psrch_inf);
__u16 searchHandle, struct cifs_search_info * psrch_inf);

extern int CIFSFindClose(const int, struct cifsTconInfo *tcon,
const __u16 search_handle);
Expand Down Expand Up @@ -230,8 +231,9 @@ extern int CIFSSMBClose(const int xid, struct cifsTconInfo *tcon,
const int smb_file_id);

extern int CIFSSMBRead(const int xid, struct cifsTconInfo *tcon,
const int netfid, unsigned int count,
const __u64 lseek, unsigned int *nbytes, char **buf);
const int netfid, unsigned int count,
const __u64 lseek, unsigned int *nbytes, char **buf,
int * return_buf_type);
extern int CIFSSMBWrite(const int xid, struct cifsTconInfo *tcon,
const int netfid, const unsigned int count,
const __u64 lseek, unsigned int *nbytes,
Expand Down
93 changes: 58 additions & 35 deletions fs/cifs/cifssmb.c
Original file line number Diff line number Diff line change
Expand Up @@ -958,21 +958,19 @@ CIFSSMBOpen(const int xid, struct cifsTconInfo *tcon,
return rc;
}

/* If no buffer passed in, then caller wants to do the copy
as in the case of readpages so the SMB buffer must be
freed by the caller */

int
CIFSSMBRead(const int xid, struct cifsTconInfo *tcon,
const int netfid, const unsigned int count,
const __u64 lseek, unsigned int *nbytes, char **buf)
const int netfid, const unsigned int count,
const __u64 lseek, unsigned int *nbytes, char **buf,
int * pbuf_type)
{
int rc = -EACCES;
READ_REQ *pSMB = NULL;
READ_RSP *pSMBr = NULL;
char *pReadData = NULL;
int bytes_returned;
int wct;
int resp_buf_type = 0;
struct kvec iov[1];

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

*nbytes = 0;
rc = smb_init(SMB_COM_READ_ANDX, wct, tcon, (void **) &pSMB,
(void **) &pSMBr);
rc = small_smb_init(SMB_COM_READ_ANDX, wct, tcon, (void **) &pSMB);
if (rc)
return rc;

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

pSMB->AndXCommand = 0xFF; /* none */
pSMB->AndXCommand = 0xFF; /* none */
pSMB->Fid = netfid;
pSMB->OffsetLow = cpu_to_le32(lseek & 0xFFFFFFFF);
if(wct == 12)
pSMB->OffsetHigh = cpu_to_le32(lseek >> 32);
else if((lseek >> 32) > 0) /* can not handle this big offset for old */
return -EIO;
else if((lseek >> 32) > 0) /* can not handle this big offset for old */
return -EIO;

pSMB->Remaining = 0;
pSMB->MaxCount = cpu_to_le16(count & 0xFFFF);
Expand All @@ -1005,14 +1002,18 @@ CIFSSMBRead(const int xid, struct cifsTconInfo *tcon,
pSMB->ByteCount = 0; /* no need to do le conversion since 0 */
else {
/* old style read */
struct smb_com_readx_req * pSMBW =
struct smb_com_readx_req * pSMBW =
(struct smb_com_readx_req *)pSMB;
pSMBW->ByteCount = 0;
pSMBW->ByteCount = 0;
}

rc = SendReceive(xid, tcon->ses, (struct smb_hdr *) pSMB,
(struct smb_hdr *) pSMBr, &bytes_returned, 0);

iov[0].iov_base = (char *)pSMB;
iov[0].iov_len = pSMB->hdr.smb_buf_length + 4;
rc = SendReceive2(xid, tcon->ses, iov,
1 /* num iovecs */,
&resp_buf_type, 0);
cifs_stats_inc(&tcon->num_reads);
pSMBr = (READ_RSP *)iov[0].iov_base;
if (rc) {
cERROR(1, ("Send error in read = %d", rc));
} else {
Expand All @@ -1022,33 +1023,43 @@ CIFSSMBRead(const int xid, struct cifsTconInfo *tcon,
*nbytes = data_length;

/*check that DataLength would not go beyond end of SMB */
if ((data_length > CIFSMaxBufSize)
if ((data_length > CIFSMaxBufSize)
|| (data_length > count)) {
cFYI(1,("bad length %d for count %d",data_length,count));
rc = -EIO;
*nbytes = 0;
} else {
pReadData =
(char *) (&pSMBr->hdr.Protocol) +
pReadData = (char *) (&pSMBr->hdr.Protocol) +
le16_to_cpu(pSMBr->DataOffset);
/* if(rc = copy_to_user(buf, pReadData, data_length)) {
cERROR(1,("Faulting on read rc = %d",rc));
rc = -EFAULT;
}*/ /* can not use copy_to_user when using page cache*/
/* if(rc = copy_to_user(buf, pReadData, data_length)) {
cERROR(1,("Faulting on read rc = %d",rc));
rc = -EFAULT;
}*/ /* can not use copy_to_user when using page cache*/
if(*buf)
memcpy(*buf,pReadData,data_length);
memcpy(*buf,pReadData,data_length);
}
}
if(*buf)
cifs_buf_release(pSMB);
else
*buf = (char *)pSMB;

/* Note: On -EAGAIN error only caller can retry on handle based calls
cifs_small_buf_release(pSMB);
if(*buf) {
if(resp_buf_type == CIFS_SMALL_BUFFER)
cifs_small_buf_release(iov[0].iov_base);
else if(resp_buf_type == CIFS_LARGE_BUFFER)
cifs_buf_release(iov[0].iov_base);
} else /* return buffer to caller to free */ /* BB FIXME how do we tell caller if it is not a large buffer */ {
*buf = iov[0].iov_base;
if(resp_buf_type == CIFS_SMALL_BUFFER)
*pbuf_type = CIFS_SMALL_BUFFER;
else if(resp_buf_type == CIFS_LARGE_BUFFER)
*pbuf_type = CIFS_LARGE_BUFFER;
}

/* Note: On -EAGAIN error only caller can retry on handle based calls
since file handle passed in no longer valid */
return rc;
}


int
CIFSSMBWrite(const int xid, struct cifsTconInfo *tcon,
const int netfid, const unsigned int count,
Expand Down Expand Up @@ -1163,10 +1174,10 @@ CIFSSMBWrite2(const int xid, struct cifsTconInfo *tcon,
{
int rc = -EACCES;
WRITE_REQ *pSMB = NULL;
int bytes_returned, wct;
int wct;
int smb_hdr_len;
int resp_buf_type = 0;

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

if(tcon->ses->capabilities & CAP_LARGE_FILES)
Expand Down Expand Up @@ -1209,22 +1220,34 @@ CIFSSMBWrite2(const int xid, struct cifsTconInfo *tcon,
pSMBW->ByteCount = cpu_to_le16(count + 5);
}
iov[0].iov_base = pSMB;
iov[0].iov_len = smb_hdr_len + 4;
if(wct == 14)
iov[0].iov_len = smb_hdr_len + 4;
else /* wct == 12 pad bigger by four bytes */
iov[0].iov_len = smb_hdr_len + 8;


rc = SendReceive2(xid, tcon->ses, iov, n_vec + 1, &bytes_returned,
rc = SendReceive2(xid, tcon->ses, iov, n_vec + 1, &resp_buf_type,
long_op);
cifs_stats_inc(&tcon->num_writes);
if (rc) {
cFYI(1, ("Send error Write2 = %d", rc));
*nbytes = 0;
} else if(resp_buf_type == 0) {
/* presumably this can not happen, but best to be safe */
rc = -EIO;
*nbytes = 0;
} else {
WRITE_RSP * pSMBr = (WRITE_RSP *)pSMB;
WRITE_RSP * pSMBr = (WRITE_RSP *)iov[0].iov_base;
*nbytes = le16_to_cpu(pSMBr->CountHigh);
*nbytes = (*nbytes) << 16;
*nbytes += le16_to_cpu(pSMBr->Count);
}

cifs_small_buf_release(pSMB);
if(resp_buf_type == CIFS_SMALL_BUFFER)
cifs_small_buf_release(iov[0].iov_base);
else if(resp_buf_type == CIFS_LARGE_BUFFER)
cifs_buf_release(iov[0].iov_base);

/* Note: On -EAGAIN error only caller can retry on handle based calls
since file handle passed in no longer valid */
Expand Down
2 changes: 1 addition & 1 deletion fs/cifs/connect.c
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,7 @@ cifs_demultiplex_thread(struct TCP_Server_Info *server)
/* else length ok */
reconnect = 0;

if(pdu_length > MAX_CIFS_HDR_SIZE - 4) {
if(pdu_length > MAX_CIFS_SMALL_BUFFER_SIZE - 4) {
isLargeBuf = TRUE;
memcpy(bigbuf, smallbuf, 4);
smb_buffer = bigbuf;
Expand Down
Loading

0 comments on commit ec637e3

Please sign in to comment.