Re: xattr patches from feb?

Re: xattr patches from feb?

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1023 bytes --]

On 06/24/2013 01:15 PM, Mats Kärrman wrote:
> Sorry for contacting you directly.

I've added linux-mtd on Cc, maybe there are more people interested in
the patches.

> I have been running integck on UBIFS and found some issues with
> corrupt xattr nodes. I found your post on the linux-mtd mailing list
> but nothing seem to have happened since February.

Yes, unfortunately.

> What has happened since? Were the patches rejected or just ignored? 
> Do you still believe in them?

Nothing happened, our $CUSTOMER shifted priorities. There is a locking
issue with the patches. Which might be a false positive by the lockdep.

I'll send you my current patches as PM, if someone else is interested,
contact me.

regards,
Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

RE: xattr patches from feb?

[Mats Kärrman]

Hi Marc,

Thanks for your response! it was held captive by the spam filter for a while (my fault) so I didn't notice it.
I know about customers and changing priorities ;)

I havn't received any patches from you yet (also a mail program issue?).
I have only adopted 1/5 "xattr: protect ui_size and data_len by ui_mutex" and 2/5 "fix couple bugs in
UBIFS extended attribute len" (appended below).
Those two seem to work well (testing still in progress) and should not be affected by the locking issue?

BR // Mats

---------------------------
diff -pruN a/fs/ubifs/journal.c b/fs/ubifs/journal.c
--- a/fs/ubifs/journal.c	2013-06-24 15:06:39.213689332 +0200
+++ b/fs/ubifs/journal.c	2013-06-24 15:26:36.842825120 +0200
@@ -553,7 +553,8 @@ int ubifs_jnl_update(struct ubifs_info *
 
 	dbg_jnl("ino %lu, dent '%.*s', data len %d in dir ino %lu",
 		inode->i_ino, nm->len, nm->name, ui->data_len, dir->i_ino);
-	ubifs_assert(dir_ui->data_len == 0);
+	if (!xent)
+		ubifs_assert(dir_ui->data_len == 0);
 	ubifs_assert(mutex_is_locked(&dir_ui->ui_mutex));
 
 	dlen = UBIFS_DENT_NODE_SZ + nm->len + 1;
@@ -573,6 +574,8 @@ int ubifs_jnl_update(struct ubifs_info *
 	aligned_dlen = ALIGN(dlen, 8);
 	aligned_ilen = ALIGN(ilen, 8);
 	len = aligned_dlen + aligned_ilen + UBIFS_INO_NODE_SZ;
+	len = aligned_dlen + aligned_ilen + UBIFS_INO_NODE_SZ +
+	      dir_ui->data_len;
 	dent = kmalloc(len, GFP_NOFS);
 	if (!dent)
 		return -ENOMEM;
@@ -649,7 +652,8 @@ int ubifs_jnl_update(struct ubifs_info *
 
 	ino_key_init(c, &ino_key, dir->i_ino);
 	ino_offs += aligned_ilen;
-	err = ubifs_tnc_add(c, &ino_key, lnum, ino_offs, UBIFS_INO_NODE_SZ);
+	err = ubifs_tnc_add(c, &ino_key, lnum, ino_offs,
+			    UBIFS_INO_NODE_SZ + dir_ui->data_len);
 	if (err)
 		goto out_ro;
 
diff -pruN a/fs/ubifs/xattr.c b/fs/ubifs/xattr.c
--- a/fs/ubifs/xattr.c	2013-06-24 15:06:39.233689282 +0200
+++ b/fs/ubifs/xattr.c	2013-06-24 15:12:12.196863493 +0200
@@ -143,10 +143,10 @@ static int create_xattr(struct ubifs_inf
 		err = -ENOMEM;
 		goto out_free;
 	}
-	inode->i_size = ui->ui_size = size;
-	ui->data_len = size;
 
 	mutex_lock(&host_ui->ui_mutex);
+	inode->i_size = ui->ui_size = size;
+	ui->data_len = size;
 	host->i_ctime = ubifs_current_time(host);
 	host_ui->xattr_cnt += 1;
 	host_ui->xattr_size += CALC_DENT_SIZE(nm->len);
@@ -208,10 +208,10 @@ static int change_xattr(struct ubifs_inf
 		err = -ENOMEM;
 		goto out_free;
 	}
-	inode->i_size = ui->ui_size = size;
-	ui->data_len = size;
 
 	mutex_lock(&host_ui->ui_mutex);
+	inode->i_size = ui->ui_size = size;
+	ui->data_len = size;
 	host->i_ctime = ubifs_current_time(host);
 	host_ui->xattr_size -= CALC_XATTR_BYTES(ui->data_len);
 	host_ui->xattr_size += CALC_XATTR_BYTES(size);
---------------------------

Re: xattr patches from feb?

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1344 bytes --]

On 06/27/2013 11:37 AM, Mats Kärrman wrote:
> Thanks for your response! it was held captive by the spam filter for
> a while (my fault) so I didn't notice it. I know about customers and
> changing priorities ;)
> 
> I havn't received any patches from you yet (also a mail program
> issue?).

Maybe, I'll push the patches to our git server:

http://git.pengutronix.de/?p=mkl/linux.git;a=shortlog;h=refs/heads/v3.10/topic/ubifs-xattr

> I have only adopted 1/5 "xattr: protect ui_size and data_len by ui_mutex" and 2/5 "fix couple bugs in
> UBIFS extended attribute len" (appended below).
> Those two seem to work well (testing still in progress) and should not be affected by the locking issue?

Can you please (re)check if these patches are still the ones on my
server? Can I have your Tested-by? Do they fix your problems? If so, we
should try to get them mainline.

And yes, the first two patches are unrelated to locking problem. The
locking problems are "introduced" by "UBIFS: Add security.* XATTR
support for the UBIFS" I think.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

RE: xattr patches from feb?

[Mats Kärrman]

On Thu, June 27, 2013 11:51 AM, Marc Kleine-Budde [mkl@pengutronix.de] wrote:
> Can you please (re)check if these patches are still the ones on my
> server?

Yes, my patch is equivalent to your committs
1a5ef9e82c7c76a90de94190df6d0ad9cfad2452
and
88dfd5c021b6a628a3b51e8f9f34e4d26a01f4f4

> Can I have your Tested-by? Do they fix your problems?

I'm still testing but so far it looks promising.

> If so, we should try to get them mainline.

Sounds like a great idea!

// Mats

RE: xattr patches from feb?

[Mats Kärrman]

Hi Marc,

I have now run your patches both on the bench using the integck test program and
in live systems for an extended period of time. The xattr node related errors I saw
before has not appeared any more.
All tests run on a MPC5125 based system using NOR FLASH.

Tested-by: Mats Karrman <mats.karrman@tritech.se>

BR // Mats
________________________________________
From: linux-mtd [linux-mtd-bounces@lists.infradead.org] on behalf of Mats Kärrman [Mats.Karrman@tritech.se]
Sent: Thursday, June 27, 2013 12:59 PM
To: Marc Kleine-Budde
Cc: linux-mtd
Subject: RE: xattr patches from feb?

On Thu, June 27, 2013 11:51 AM, Marc Kleine-Budde [mkl@pengutronix.de] wrote:
> Can you please (re)check if these patches are still the ones on my
> server?

Yes, my patch is equivalent to your committs
1a5ef9e82c7c76a90de94190df6d0ad9cfad2452
and
88dfd5c021b6a628a3b51e8f9f34e4d26a01f4f4

> Can I have your Tested-by? Do they fix your problems?

I'm still testing but so far it looks promising.

> If so, we should try to get them mainline.

Sounds like a great idea!

// Mats
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/