Skip to contentSkip to conversation

CIFS: use the correct length when pinning memory for direct I/O for write

PATCH Applied Long Li 4 participants 4 messages Dec 16, 2018 – Dec 18, 2018

Subsystems (4)
Stable branch InfiniBand SMB client (CIFS) Kernel SMB3 server (ksmbd)
lore
Share & tools
lore

Long Li

author
#0 lore raw

PATCH 1/1 CIFS: use the correct length when pinning memory for direct I/O for write

From: Long Li <[email protected]>

The current code attempts to pin memory using the largest possible wsize based on the currect SMB credits. This doesn't cause kernel oops but this is not optimal as we may pin more pages then actually needed.

Fix this by only pinning what are needed for doing this write I/O.

Signed-off-by Long Li <[email protected]>
Cc [email protected]
---
 fs/cifs/file.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
fs/cifs/file.c +3 -1
index 3467351..c23bf9d 100644
@@ -2617,11 +2617,13 @@ cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from,
 		if (rc)
 			break;
 
+		cur_len = min_t(const size_t, len, wsize);
+
 		if (ctx->direct_io) {
 			ssize_t result;
 
 			result = iov_iter_get_pages_alloc(
-				from, &pagevec, wsize, &start);
+				from, &pagevec, cur_len, &start);
 			if (result < 0) {
 				cifs_dbg(VFS,
 					"direct_writev couldn't get user pages "
Signature · 1 line
2.7.4
Reviewed-by on v1 1/1
#1 lore raw
Quoted 10 lines · Long Li

From: Long Li <[email protected]>

The current code attempts to pin memory using the largest possible wsize based on the currect SMB credits. This doesn't cause kernel oops but this is not optimal as we may pin more pages then actually needed.

Fix this by only pinning what are needed for doing this write I/O.

Signed-off-by Long Li <[email protected]>
Cc [email protected]

Looks sane to me.

Reviewed-by Joey Pabalinas <[email protected]>
Quoted 26 lines
---
 fs/cifs/file.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
fs/cifs/file.c +3 -1
index 3467351..c23bf9d 100644
@@ -2617,11 +2617,13 @@ cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from,
 		if (rc)
 			break;
 
+		cur_len = min_t(const size_t, len, wsize);
+
 		if (ctx->direct_io) {
 			ssize_t result;
 
 			result = iov_iter_get_pages_alloc(
-				from, &pagevec, wsize, &start);
+				from, &pagevec, cur_len, &start);
 			if (result < 0) {
 				cifs_dbg(VFS,
 					"direct_writev couldn't get user pages "
Signature · 1 line
2.7.4
Signature · 2 lines
Cheers,
Joey Pabalinas
maintainer
#2 lore raw
In reply to Long Li

Hi,

[This is an automated email]

This commit has been processed because it contains a -stable tag. The stable tag indicates that it's relevant for the following trees: all

The bot has tested the following trees: v4.19.9, v4.14.88, v4.9.145, v4.4.167, v3.18.129,

v4.19.9: Failed to apply! Possible dependencies:
6e6e2b86c29c ("CIFS: Add support for direct I/O read") 8c5f9c1ab7cb ("CIFS: Add support for direct I/O write") b98e26df0754 ("cifs: fix signed/unsigned mismatch on aio_read patch")

v4.14.88: Failed to apply! Possible dependencies:
1dbe3466b4d2 ("CIFS: Use offset when reading pages")
6e6e2b86c29c ("CIFS: Add support for direct I/O read")
8c5f9c1ab7cb ("CIFS: Add support for direct I/O write")
b98e26df0754 ("cifs: fix signed/unsigned mismatch on aio_read patch")
bd3dcc6a22a9 ("CIFS: SMBD: Upper layer performs SMB read via RDMA write through memory registration")

v4.9.145: Failed to apply! Possible dependencies:
31473fc4f965 ("CIFS: Separate SMB2 header structure") 7fb8986e7449 ("CIFS: Add capability to transform requests before sending") 8c5f9c1ab7cb ("CIFS: Add support for direct I/O write") 9bb17e0916a0 ("CIFS: Add transform header handling callbacks") c610c4b619e5 ("CIFS: Add asynchronous write support through kernel AIO") ccf7f4088af2 ("CIFS: Add asynchronous context to support kernel AIO") da502f7df03d ("CIFS: Make SendReceive2() takes resp iov") ef65aaede23f ("smb2: Enforce sec= mount option")

v4.4.167: Failed to apply! Possible dependencies:
141891f4727c ("SMB3: Add mount parameter to allow user to override max credits")
166cea4dc3a4 ("SMB2: Separate RawNTLMSSP authentication from SMB2_sess_setup")
16c568efff82 ("cifs: merge the hash calculation helpers")
275516cdcfa4 ("Print IP address of unresponsive server")
31473fc4f965 ("CIFS: Separate SMB2 header structure")
373512ec5c10 ("Prepare for encryption support (first part). Add decryption and encryption key generation. Thanks to Metze for helping with this.") 3baf1a7b9215 ("SMB2: Separate Kerberos authentication from SMB2_sess_setup")
7fb8986e7449 ("CIFS: Add capability to transform requests before sending")
8c5f9c1ab7cb ("CIFS: Add support for direct I/O write")
9bb17e0916a0 ("CIFS: Add transform header handling callbacks")
adfeb3e00e8e ("cifs: Make echo interval tunable")
c610c4b619e5 ("CIFS: Add asynchronous write support through kernel AIO")
ccf7f4088af2 ("CIFS: Add asynchronous context to support kernel AIO")
da502f7df03d ("CIFS: Make SendReceive2() takes resp iov")
ef65aaede23f ("smb2: Enforce sec= mount option")

v3.18.129: Failed to apply! Possible dependencies:
064bcc0702a8 ("Fix coverity warning")
141891f4727c ("SMB3: Add mount parameter to allow user to override max credits")
275516cdcfa4 ("Print IP address of unresponsive server")
31473fc4f965 ("CIFS: Separate SMB2 header structure")
373512ec5c10 ("Prepare for encryption support (first part). Add decryption and encryption key generation. Thanks to Metze for helping with this.") 3baf1a7b9215 ("SMB2: Separate Kerberos authentication from SMB2_sess_setup")
5f7fbf733c9d ("Allow parsing vers=3.11 on cifs mount")
7d414f396c91 ("cifs: Limit the overall credit acquired")
7fb8986e7449 ("CIFS: Add capability to transform requests before sending")
8c5f9c1ab7cb ("CIFS: Add support for direct I/O write")
9235d0987331 ("Convert MessageID in smb2_hdr to LE")
9bb17e0916a0 ("CIFS: Add transform header handling callbacks")
aab1893d5fbe ("Add SMB3.11 mount option synonym for new dialect")
adfeb3e00e8e ("cifs: Make echo interval tunable")
b618f001a20e ("[SMB3] Enable checking for continuous availability and persistent handle support")
bc09d141ebb2 ("fs/cifs: remove obsolete __constant")
c2afb8147e69 ("Set previous session id correctly on SMB3 reconnect")
c610c4b619e5 ("CIFS: Add asynchronous write support through kernel AIO")
ccf7f4088af2 ("CIFS: Add asynchronous context to support kernel AIO")
eed0e1753cbe ("Add defines and structs for smb3.1 dialect")
ef65aaede23f ("smb2: Enforce sec= mount option")

How should we proceed with this patch?

--
Thanks,
Sasha

Applied
#3 lore raw
In reply to Long Li

merged into cifs-2.6.git for-next

Quoted 37 lines · Long Li

From: Long Li <[email protected]>

The current code attempts to pin memory using the largest possible wsize based on the currect SMB credits. This doesn't cause kernel oops but this is not optimal as we may pin more pages then actually needed.

Fix this by only pinning what are needed for doing this write I/O.

Signed-off-by Long Li <[email protected]>
Cc [email protected]
---
 fs/cifs/file.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
fs/cifs/file.c +3 -2
index 3467351..c23bf9d 100644
@@ -2617,11 +2617,13 @@ cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from,
                if (rc)
                        break;
 
+               cur_len = min_t(const size_t, len, wsize);
+
                if (ctx->direct_io) {
                        ssize_t result;
 
                        result = iov_iter_get_pages_alloc(
-                               from, &pagevec, wsize, &start);
+                               from, &pagevec, cur_len, &start);
                        if (result < 0) {
                                cifs_dbg(VFS,
                                        "direct_writev couldn't get user pages "
--

2.7.4

Signature · 3 lines
Thanks,

Steve