[PATCH 0/2] mdadm patches for r5cache and mdadm.spec

[PATCH 0/2] mdadm patches for r5cache and mdadm.spec

[Song Liu]

These two patches are not related to each other. I just found
them recently.

The first patch is found when building rpm with mdadm.spec.

The send patch fixes an --incremental issue when there is
mismatch in "Events" in an array with r5cache.

Thanks,
Song

Song Liu (2):
  mdadm: install two more udev rules in mdadm.spec
  mdadm: save previous journal_clean when reload super

 Incremental.c | 3 +++
 mdadm.spec    | 2 ++
 2 files changed, 5 insertions(+)

--
2.9.5

[PATCH 1/2] mdadm: install two more udev rules in mdadm.spec

[Song Liu]

To avoid rpmbuild error.

Signed-off-by: Song Liu <songliubraving@fb.com>
---
 mdadm.spec | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mdadm.spec b/mdadm.spec
index 4e97efb..4e6f870 100644
--- a/mdadm.spec
+++ b/mdadm.spec
@@ -37,8 +37,10 @@ rm -rf $RPM_BUILD_ROOT
 %doc TODO ChangeLog mdadm.conf-example COPYING
 %{_sbindir}/mdadm
 %{_sbindir}/mdmon
+/usr/lib/udev/rules.d/01-md-raid-creating.rules
 /usr/lib/udev/rules.d/63-md-raid-arrays.rules
 /usr/lib/udev/rules.d/64-md-raid-assembly.rules
+/usr/lib/udev/rules.d/69-md-clustered-confirm-device.rules
 %config(noreplace,missingok)/%{_sysconfdir}/mdadm.conf
 %{_mandir}/man*/md*
 
-- 
2.9.5

[PATCH 2/2] mdadm: save previous journal_clean when reload super

[Song Liu]

In Incremental.c:count_active(), max_events is tracked to show to
which devices are up to date. If a device has events==max_events+1,
getinfo_super() is called to reload the superblock from this
device. getinfo_super1() blindly set journal_clean to 0, which is
wrong. This patch fixes this issue by saving previous
journal_clean before calling getinfo_super().

Signed-off-by: Song Liu <songliubraving@fb.com>
---
 Incremental.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Incremental.c b/Incremental.c
index 6cf2174..b17b37f 100644
--- a/Incremental.c
+++ b/Incremental.c
@@ -747,13 +747,16 @@ static int count_active(struct supertype *st, struct mdinfo *sra,
 				;
 			else if (info.events == max_events+1) {
 				int i;
+				int journal_clean;
 				max_events = info.events;
 				for (i = 0; i < raid_disks; i++)
 					if (avail[i])
 						avail[i]--;
 				avail[info.disk.raid_disk] = 2;
 				best[info.disk.raid_disk] = devnum;
+				journal_clean = bestinfo->journal_clean;
 				st->ss->getinfo_super(st, bestinfo, NULL);
+				bestinfo->journal_clean = journal_clean;
 			} else { /* info.events much bigger */
 				memset(avail, 0, raid_disks);
 				max_events = info.events;
-- 
2.9.5

Re: [PATCH 2/2] mdadm: save previous journal_clean when reload super

[NeilBrown]

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

On Mon, Aug 28 2017, Song Liu wrote:

> In Incremental.c:count_active(), max_events is tracked to show to
> which devices are up to date. If a device has events==max_events+1,
> getinfo_super() is called to reload the superblock from this
> device. getinfo_super1() blindly set journal_clean to 0, which is
> wrong. This patch fixes this issue by saving previous
> journal_clean before calling getinfo_super().
>
> Signed-off-by: Song Liu <songliubraving@fb.com>
> ---
>  Incremental.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/Incremental.c b/Incremental.c
> index 6cf2174..b17b37f 100644
> --- a/Incremental.c
> +++ b/Incremental.c
> @@ -747,13 +747,16 @@ static int count_active(struct supertype *st, struct mdinfo *sra,
>  				;
>  			else if (info.events == max_events+1) {
>  				int i;
> +				int journal_clean;
>  				max_events = info.events;
>  				for (i = 0; i < raid_disks; i++)
>  					if (avail[i])
>  						avail[i]--;
>  				avail[info.disk.raid_disk] = 2;
>  				best[info.disk.raid_disk] = devnum;
> +				journal_clean = bestinfo->journal_clean;
>  				st->ss->getinfo_super(st, bestinfo, NULL);
> +				bestinfo->journal_clean = journal_clean;
>  			} else { /* info.events much bigger */
>  				memset(avail, 0, raid_disks);
>  				max_events = info.events;


This is a hack to work around a symptom.  It is not a real fix.

I'm not sure what the real fix is.
Maybe when you find "raid_disk == MD_DISK_ROLE_JOURNAL" you set
"journal_events = info.events", then after the loop, if
"journal_events >= max_events-1", you set "bestinfo->journal_clean = 1",
or something like that.
But you need to make it obvious that the code is correct, and the above
code looks like a hack.

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH 0/2] mdadm patches for r5cache and mdadm.spec

[Jes Sorensen]

On 08/28/2017 06:20 PM, Song Liu wrote:
> These two patches are not related to each other. I just found
> them recently.
> 
> The first patch is found when building rpm with mdadm.spec.
> 
> The send patch fixes an --incremental issue when there is
> mismatch in "Events" in an array with r5cache.
> 
> Thanks,
> Song
> 
> Song Liu (2):
>    mdadm: install two more udev rules in mdadm.spec
>    mdadm: save previous journal_clean when reload super
> 
>   Incremental.c | 3 +++
>   mdadm.spec    | 2 ++
>   2 files changed, 5 insertions(+)

Hi,

I applied the first one, but leaving the second one out for now, per 
Neil's comments.

Cheers,
Jes