Christopher R. Hertel | 1 Mar 20:18 2012
Picon

Re: [PATCH 06/11] CIFS: Respect MaxMpxCount field

On 03/01/2012 10:48 AM, Jeremy Allison wrote:
> On Thu, Mar 01, 2012 at 01:37:47PM -0500, Jeff Layton wrote:
>>
>> (cc'ing Chris and Jeremy to make sure I understand)
>>
>> The description above is a good start but doesn't quite outline the
>> clear concise "rules" for this that I was looking for.
>>
>> So if I understand what Steve wrote above, he's basically saying that
>> we should enforce the maxmpx for everything but blocking locks and
>> echoes? Is that correct? If so then I think that's wrong and won't fix
>> any of the problems that people have reported with the existing code.
>>
>> It's all well and good that *some* servers allow you to exceed the
>> maxmpx in *some* cases, but we can't code to that assumption. We know
>> well that many servers do not handle exceeding the maxmpx gracefully at
>> all. As always we have to code to the lowest common denominator by
>> default, and then optionally allow people to exceed that if they choose
>> to do so.
>>
>> I think this is a case where we need a good description in a human
>> language (preferably english) of how this should work (aka a
>> specification), and then write code to match that description (aka code
>> to the spec).
>>
>> Anything else is going to send us down the rabbit hole where we are
>> today with cifs.ko -- a bunch of ad-hoc, broken code that no one really
>> understands.
>>
>> While we may not like it, a hard cap on the number of outstanding
(Continue reading)

Jesper Juhl | 1 Mar 21:45 2012
Picon

[PATCH] cifs: don't dereference potentially NULL variable in match_session()

This bit of code:
...
		if (strncmp(ses->user_name,
			    vol->username ? vol->username : "",
			    MAX_USERNAME_SIZE))
			return 0;
...
implies that 'vol->username' may be NULL.
If it is NULL, then the 'strlen(vol->username)' that follows will
dereference a NULL pointer.

This patch should take care of that issue.

Signed-off-by: Jesper Juhl <jj@...>
---
 fs/cifs/connect.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

 Compile tested only.

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 602f77c..2f3cf02 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
 <at>  <at>  -2014,8 +2014,9  <at>  <at>  static int match_session(struct cifs_ses *ses, struct smb_vol *vol)
 			    vol->username ? vol->username : "",
 			    MAX_USERNAME_SIZE))
 			return 0;
-		if (strlen(vol->username) != 0 &&
-		    ses->password != NULL &&
(Continue reading)

Steve French | 1 Mar 23:30 2012
Picon

MaxMpx and Blocking Locks

FYI - There was followup discussion after the dochelp response,
explaining some of the quirks, and some still open questions (about
why Windows doesn't return an error as implied, for example).

My concern remains that we don't want to make things worse with the
fix (opening up deadlocks or data integrity issues) so I would prefer
to  allow SMB Echo through (since it works to all known servers with
MaxMpx 10 or greater, and the alternative, not sending it, may cause
the session to drop and make things worse). Chris Hertel had an
interesting suggestion (he may be able to describe it in more detail)
though for limiting blocking locks to avoid blocking locks "starving"
the available mpx (which are needed for read and write etc.).   I had
suggested limiting outstanding blocking locks to MaxMpx - 2 (or
something similar) - Chris suggested that the client instead use
MaxMpx/2 as a maximum for blocking locks to avoid "starving" the
system (and e.g. deadlocking when we have to do writeback).  This
would involve us keeping a counter of outstanding blocking locks in
the session structure.  This shouldn't slow performance much because a
simple atomic counter could be used and is probably good enough for an
approximation of the current pending locks (it would not require an
expensive semaphore or spinlock).

--

-- 
Thanks,

Steve
Ted Ts'o | 2 Mar 00:29 2012
Picon
Picon

Re: [PATCH 00/11 v2] Push file_update_time() into .page_mkwrite

On Thu, Mar 01, 2012 at 12:41:34PM +0100, Jan Kara wrote:
> 
> To fix the issue, this patch set changes page fault code to call
> file_update_time() only when ->page_mkwrite() callback is not provided. If the
> callback is provided, it is the responsibility of the filesystem to perform
> update of i_mtime / i_ctime if needed. We also push file_update_time() call
> to all existing ->page_mkwrite() implementations if the time update does not
> obviously happen by other means. If you know your filesystem does not need
> update of modification times in ->page_mkwrite() handler, please speak up and
> I'll drop the patch for your filesystem.

I don't know if this introductory text is going to be saved anywhere
permanent, such as the merge commit (since git now has the ability to
have much more informative merge descriptions).  But if it is going to
be preserved, it might be worth mentioning that if the filesystem uses
block_page_mkpage(), it will handled automatically for them since the
patch series does push the call to file_update_time(0 into
__block_page_mkpage().

					- Ted

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo <at> kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont <at> kvack.org"> email <at> kvack.org </a>

Steve French | 2 Mar 02:03 2012
Picon

(unknown)

Turns out it is easy to negotiate SMB2.2 against Windows 8 (rather
than just 2.1 or 2.0 as Pavel's current git tree does).  Basic
operations worked fine.  SMB2.2 is a very cool protocol, but since
most features are optional, upgrading to SMB2.2 is not bad.  For
example this change to Pavel's git tree is trivial:

diff -U 4 fs/smb2/smb2pdu.c fs/cifs/smb2pdu.c
--- fs/smb2/smb2pdu.c	2012-03-01 18:30:24.986486510 -0600
+++ fs/cifs/smb2pdu.c	2012-03-01 18:33:35.066765305 -0600
 <at>  <at>  -357,20 +357,22  <at>  <at> 
 	else if (resp_buftype == CIFS_LARGE_BUFFER)
 		cifs_buf_release(pSMB2r);
 }

-#define SMB2_NUM_PROT 2
+#define SMB2_NUM_PROT 3

 #define SMB2_PROT   0
 #define SMB21_PROT  1
+#define SMB22_PROT  2
 #define BAD_PROT 0xFFFF

 static struct {
 	int index;
 	__le16 name;
 } smb2protocols[] = {
 	{SMB2_PROT,  cpu_to_le16(SMB2_PROT_ID)},
 	{SMB21_PROT, cpu_to_le16(SMB21_PROT_ID)},
+	{SMB22_PROT, cpu_to_le16(SMB22_PROT_ID)},
 	{BAD_PROT,   cpu_to_le16(BAD_PROT_ID)}
(Continue reading)

Steve French | 2 Mar 02:06 2012
Picon

SMB2.2 Negotiation and typical operations work to Windows 8 from Pavel's current patch set

Forgot to include subject line

Functional testing went pretty well this week.  I did get a test
failure on one of the connectathon lock tests and one of the "special"
tests, and I didn't turn on cifsacl support, but nothing critical.

---------- Forwarded message ----------
From: Steve French <smfrench@...>
Date: Thu, Mar 1, 2012 at 7:03 PM
Subject:
To: Shirish Pargaonkar <shirishpargaonkar@...>, Pavel Shilovsky
<piastryyy@...>, linux-cifs@...

Turns out it is easy to negotiate SMB2.2 against Windows 8 (rather
than just 2.1 or 2.0 as Pavel's current git tree does).  Basic
operations worked fine.  SMB2.2 is a very cool protocol, but since
most features are optional, upgrading to SMB2.2 is not bad.  For
example this change to Pavel's git tree is trivial:

diff -U 4 fs/smb2/smb2pdu.c fs/cifs/smb2pdu.c
--- fs/smb2/smb2pdu.c   2012-03-01 18:30:24.986486510 -0600
+++ fs/cifs/smb2pdu.c   2012-03-01 18:33:35.066765305 -0600
 <at>  <at>  -357,20 +357,22  <at>  <at> 
       else if (resp_buftype == CIFS_LARGE_BUFFER)
               cifs_buf_release(pSMB2r);
 }

-#define SMB2_NUM_PROT 2
+#define SMB2_NUM_PROT 3

(Continue reading)

santosh nayak | 2 Mar 07:17 2012
Picon

[PATCH] cifs: possible memory leak in xattr.

From: Santosh Nayak <santoshprasadnayak@...>

Memory is allocated irrespective of whether CIFS_ACL is configured
or not. But free is happenning only if CIFS_ACL is set. This is a
possible memory leak scenario.

Fix is:
Allocate and free memory only if CIFS_ACL is configured.

Signed-off-by: Santosh Nayak <santoshprasadnayak@...>
---
 fs/cifs/xattr.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/cifs/xattr.c b/fs/cifs/xattr.c
index 45f07c4..10d92cf 100644
--- a/fs/cifs/xattr.c
+++ b/fs/cifs/xattr.c
 <at>  <at>  -105,7 +105,6  <at>  <at>  int cifs_setxattr(struct dentry *direntry, const char *ea_name,
 	struct cifs_tcon *pTcon;
 	struct super_block *sb;
 	char *full_path;
-	struct cifs_ntsd *pacl;

 	if (direntry == NULL)
 		return -EIO;
 <at>  <at>  -164,23 +163,24  <at>  <at>  int cifs_setxattr(struct dentry *direntry, const char *ea_name,
 			cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
 	} else if (strncmp(ea_name, CIFS_XATTR_CIFS_ACL,
 			strlen(CIFS_XATTR_CIFS_ACL)) == 0) {
(Continue reading)

Jan Kara | 2 Mar 10:41 2012
Picon

Re: [PATCH 00/11 v2] Push file_update_time() into .page_mkwrite

On Thu 01-03-12 18:29:42, Ted Tso wrote:
> On Thu, Mar 01, 2012 at 12:41:34PM +0100, Jan Kara wrote:
> > 
> > To fix the issue, this patch set changes page fault code to call
> > file_update_time() only when ->page_mkwrite() callback is not provided. If the
> > callback is provided, it is the responsibility of the filesystem to perform
> > update of i_mtime / i_ctime if needed. We also push file_update_time() call
> > to all existing ->page_mkwrite() implementations if the time update does not
> > obviously happen by other means. If you know your filesystem does not need
> > update of modification times in ->page_mkwrite() handler, please speak up and
> > I'll drop the patch for your filesystem.
> 
> I don't know if this introductory text is going to be saved anywhere
> permanent, such as the merge commit (since git now has the ability to
> have much more informative merge descriptions).  But if it is going to
> be preserved, it might be worth mentioning that if the filesystem uses
> block_page_mkpage(), it will handled automatically for them since the
> patch series does push the call to file_update_time(0 into
> __block_page_mkpage().
  Good point, added to description.

								Honza
--

-- 
Jan Kara <jack <at> suse.cz>
SUSE Labs, CR

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo <at> kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
(Continue reading)

Jens Axboe | 2 Mar 10:51 2012
Picon

Re: [PATCH] Block: use a freezable workqueue for disk-event polling

On 02/17/2012 10:22 PM, Alan Stern wrote:
> This patch (as1519) fixes a bug in the block layer's disk-events
> polling.  The polling is done by a work routine queued on the
> system_nrt_wq workqueue.  Since that workqueue isn't freezable, the
> polling continues even in the middle of a system sleep transition.
> 
> Obviously, polling a suspended drive for media changes and such isn't
> a good thing to do; in the case of USB mass-storage devices it can
> lead to real problems requiring device resets and even re-enumeration.
> 
> The patch fixes things by creating a new system-wide, non-reentrant,
> freezable workqueue and using it for disk-events polling.

Thanks Alan, picked up.

--

-- 
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Jeff Layton | 2 Mar 16:45 2012
Picon

Re: [PATCH 06/11] CIFS: Respect MaxMpxCount field

On Thu, 01 Mar 2012 11:18:01 -0800
"Christopher R. Hertel" <crh@...> wrote:

> On 03/01/2012 10:48 AM, Jeremy Allison wrote:
> > On Thu, Mar 01, 2012 at 01:37:47PM -0500, Jeff Layton wrote:
> >>
> >> (cc'ing Chris and Jeremy to make sure I understand)
> >>
> >> The description above is a good start but doesn't quite outline the
> >> clear concise "rules" for this that I was looking for.
> >>
> >> So if I understand what Steve wrote above, he's basically saying that
> >> we should enforce the maxmpx for everything but blocking locks and
> >> echoes? Is that correct? If so then I think that's wrong and won't fix
> >> any of the problems that people have reported with the existing code.
> >>
> >> It's all well and good that *some* servers allow you to exceed the
> >> maxmpx in *some* cases, but we can't code to that assumption. We know
> >> well that many servers do not handle exceeding the maxmpx gracefully at
> >> all. As always we have to code to the lowest common denominator by
> >> default, and then optionally allow people to exceed that if they choose
> >> to do so.
> >>
> >> I think this is a case where we need a good description in a human
> >> language (preferably english) of how this should work (aka a
> >> specification), and then write code to match that description (aka code
> >> to the spec).
> >>
> >> Anything else is going to send us down the rabbit hole where we are
> >> today with cifs.ko -- a bunch of ad-hoc, broken code that no one really
(Continue reading)


Gmane