* [PATCH] pstore: avoid incorrectly mark entry as duplicate @ 2013-10-23 14:55 ` Madper Xie 0 siblings, 0 replies; 10+ messages in thread From: Madper Xie @ 2013-10-23 14:55 UTC (permalink / raw) To: Luck, Tony, Anton Vorontsov, linux-efi@vger.kernel.org, linux-kernel@vger.kernel.org, Seiji Aguchi, Lenny Szubowicz Cc: 成骏 谢 pstore try to find duplicate entries by check both ID, type and psi. They are not really enough for efi backend. dumped vars always have the same type, psi and ID. like follows: dump-type0-9-1-1382511508-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0 dump-type0-9-1-1382515661-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0 The "duplicate" entries won't appear in pstorefs. And a complain will be print -- pstore: failed to load 76 record(s) from 'efi' So I add one more check: timespec. Signed-off-by: Madper Xie <cxie-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> --- fs/pstore/inode.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c index 1282384..f70f1e5 100644 --- a/fs/pstore/inode.c +++ b/fs/pstore/inode.c @@ -47,6 +47,7 @@ static LIST_HEAD(allpstore); struct pstore_private { struct list_head list; struct pstore_info *psi; + struct timespec time; enum pstore_type_id type; u64 id; int count; @@ -290,7 +291,8 @@ int pstore_mkfile(enum pstore_type_id type, char *psname, u64 id, int count, list_for_each_entry(pos, &allpstore, list) { if (pos->type == type && pos->id == id && - pos->psi == psi) { + pos->psi == psi && + !timespec_compare(&pos->time, &time)) { rc = -EEXIST; break; } @@ -312,6 +314,7 @@ int pstore_mkfile(enum pstore_type_id type, char *psname, u64 id, int count, private->id = id; private->count = count; private->psi = psi; + memcpy(&private->time, &time, sizeof(struct timespec)); switch (type) { case PSTORE_TYPE_DMESG: -- 1.8.4.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] pstore: avoid incorrectly mark entry as duplicate @ 2013-10-23 14:55 ` Madper Xie 0 siblings, 0 replies; 10+ messages in thread From: Madper Xie @ 2013-10-23 14:55 UTC (permalink / raw) To: Luck, Tony, Anton Vorontsov, linux-efi@vger.kernel.org, linux-kernel@vger.kernel.org, Seiji Aguchi, Lenny Szubowicz Cc: 成骏 谢 pstore try to find duplicate entries by check both ID, type and psi. They are not really enough for efi backend. dumped vars always have the same type, psi and ID. like follows: dump-type0-9-1-1382511508-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0 dump-type0-9-1-1382515661-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0 The "duplicate" entries won't appear in pstorefs. And a complain will be print -- pstore: failed to load 76 record(s) from 'efi' So I add one more check: timespec. Signed-off-by: Madper Xie <cxie@redhat.com> --- fs/pstore/inode.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c index 1282384..f70f1e5 100644 --- a/fs/pstore/inode.c +++ b/fs/pstore/inode.c @@ -47,6 +47,7 @@ static LIST_HEAD(allpstore); struct pstore_private { struct list_head list; struct pstore_info *psi; + struct timespec time; enum pstore_type_id type; u64 id; int count; @@ -290,7 +291,8 @@ int pstore_mkfile(enum pstore_type_id type, char *psname, u64 id, int count, list_for_each_entry(pos, &allpstore, list) { if (pos->type == type && pos->id == id && - pos->psi == psi) { + pos->psi == psi && + !timespec_compare(&pos->time, &time)) { rc = -EEXIST; break; } @@ -312,6 +314,7 @@ int pstore_mkfile(enum pstore_type_id type, char *psname, u64 id, int count, private->id = id; private->count = count; private->psi = psi; + memcpy(&private->time, &time, sizeof(struct timespec)); switch (type) { case PSTORE_TYPE_DMESG: -- 1.8.4.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] pstore: avoid incorrectly mark entry as duplicate 2013-10-23 14:55 ` Madper Xie (?) @ 2013-10-23 15:58 ` Richard Weinberger [not found] ` <CAFLxGvxT1S3km=LordqFGbAb_pWc0cf3oE2ciS9uY4BBJJ231A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> -1 siblings, 1 reply; 10+ messages in thread From: Richard Weinberger @ 2013-10-23 15:58 UTC (permalink / raw) To: Madper Xie Cc: Luck, Tony, Anton Vorontsov, linux-efi@vger.kernel.org, linux-kernel@vger.kernel.org, Seiji Aguchi, Lenny Szubowicz, 成骏 谢 On Wed, Oct 23, 2013 at 4:55 PM, Madper Xie <cxie@redhat.com> wrote: > pstore try to find duplicate entries by check both ID, type and psi. > They are not really enough for efi backend. dumped vars always have > the same type, psi and ID. like follows: > dump-type0-9-1-1382511508-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0 > dump-type0-9-1-1382515661-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0 > > The "duplicate" entries won't appear in pstorefs. And a complain will be > print -- pstore: failed to load 76 record(s) from 'efi' > So I add one more check: timespec. > > Signed-off-by: Madper Xie <cxie@redhat.com> > --- > fs/pstore/inode.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c > index 1282384..f70f1e5 100644 > --- a/fs/pstore/inode.c > +++ b/fs/pstore/inode.c > @@ -47,6 +47,7 @@ static LIST_HEAD(allpstore); > struct pstore_private { > struct list_head list; > struct pstore_info *psi; > + struct timespec time; > enum pstore_type_id type; > u64 id; > int count; > @@ -290,7 +291,8 @@ int pstore_mkfile(enum pstore_type_id type, char *psname, u64 id, int count, > list_for_each_entry(pos, &allpstore, list) { > if (pos->type == type && > pos->id == id && > - pos->psi == psi) { > + pos->psi == psi && > + !timespec_compare(&pos->time, &time)) { > rc = -EEXIST; > break; > } > @@ -312,6 +314,7 @@ int pstore_mkfile(enum pstore_type_id type, char *psname, u64 id, int count, > private->id = id; > private->count = count; > private->psi = psi; > + memcpy(&private->time, &time, sizeof(struct timespec)); > > switch (type) { > case PSTORE_TYPE_DMESG: As discussed on IRC, why don't you compare the variable names? -- Thanks, //richard ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <CAFLxGvxT1S3km=LordqFGbAb_pWc0cf3oE2ciS9uY4BBJJ231A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH] pstore: avoid incorrectly mark entry as duplicate 2013-10-23 15:58 ` Richard Weinberger @ 2013-10-23 16:11 ` Madper Xie 0 siblings, 0 replies; 10+ messages in thread From: Madper Xie @ 2013-10-23 16:11 UTC (permalink / raw) To: Richard Weinberger Cc: Madper Xie, Luck, Tony, Anton Vorontsov, linux-efi@vger.kernel.org, linux-kernel@vger.kernel.org, Seiji Aguchi, Lenny Szubowicz, 成骏 谢 richard.weinberger-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org writes: > On Wed, Oct 23, 2013 at 4:55 PM, Madper Xie <cxie-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote: >> pstore try to find duplicate entries by check both ID, type and psi. >> They are not really enough for efi backend. dumped vars always have >> the same type, psi and ID. like follows: >> dump-type0-9-1-1382511508-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0 >> dump-type0-9-1-1382515661-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0 >> >> The "duplicate" entries won't appear in pstorefs. And a complain will be >> print -- pstore: failed to load 76 record(s) from 'efi' >> So I add one more check: timespec. >> >> Signed-off-by: Madper Xie <cxie-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> >> --- >> fs/pstore/inode.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c >> index 1282384..f70f1e5 100644 >> --- a/fs/pstore/inode.c >> +++ b/fs/pstore/inode.c >> @@ -47,6 +47,7 @@ static LIST_HEAD(allpstore); >> struct pstore_private { >> struct list_head list; >> struct pstore_info *psi; >> + struct timespec time; >> enum pstore_type_id type; >> u64 id; >> int count; >> @@ -290,7 +291,8 @@ int pstore_mkfile(enum pstore_type_id type, char *psname, u64 id, int count, >> list_for_each_entry(pos, &allpstore, list) { >> if (pos->type == type && >> pos->id == id && >> - pos->psi == psi) { >> + pos->psi == psi && >> + !timespec_compare(&pos->time, &time)) { >> rc = -EEXIST; >> break; >> } >> @@ -312,6 +314,7 @@ int pstore_mkfile(enum pstore_type_id type, char *psname, u64 id, int count, >> private->id = id; >> private->count = count; >> private->psi = psi; >> + memcpy(&private->time, &time, sizeof(struct timespec)); >> >> switch (type) { >> case PSTORE_TYPE_DMESG: > > > As discussed on IRC, why don't you compare the variable names? Howdy Richard, Let's analyze the name of "duplicate" entries: >> dump-type0-9-1-1382511508-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0 >> dump-type0-9-1-1382515661-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0 The have the same name except timestamp. In this case, there is no difference between compare names and compare timestamps. ( Yeah, I'm pretty sure the guid will be the same one -- LINUX_EFI_CRASH_GUID ) But efi is only one backend. For other backends, we don't sure if they will add timestamp to name. So... -- Best, Madper Xie. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] pstore: avoid incorrectly mark entry as duplicate @ 2013-10-23 16:11 ` Madper Xie 0 siblings, 0 replies; 10+ messages in thread From: Madper Xie @ 2013-10-23 16:11 UTC (permalink / raw) To: Richard Weinberger Cc: Madper Xie, Luck, Tony, Anton Vorontsov, linux-efi@vger.kernel.org, linux-kernel@vger.kernel.org, Seiji Aguchi, Lenny Szubowicz, 成骏 谢 richard.weinberger@gmail.com writes: > On Wed, Oct 23, 2013 at 4:55 PM, Madper Xie <cxie@redhat.com> wrote: >> pstore try to find duplicate entries by check both ID, type and psi. >> They are not really enough for efi backend. dumped vars always have >> the same type, psi and ID. like follows: >> dump-type0-9-1-1382511508-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0 >> dump-type0-9-1-1382515661-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0 >> >> The "duplicate" entries won't appear in pstorefs. And a complain will be >> print -- pstore: failed to load 76 record(s) from 'efi' >> So I add one more check: timespec. >> >> Signed-off-by: Madper Xie <cxie@redhat.com> >> --- >> fs/pstore/inode.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c >> index 1282384..f70f1e5 100644 >> --- a/fs/pstore/inode.c >> +++ b/fs/pstore/inode.c >> @@ -47,6 +47,7 @@ static LIST_HEAD(allpstore); >> struct pstore_private { >> struct list_head list; >> struct pstore_info *psi; >> + struct timespec time; >> enum pstore_type_id type; >> u64 id; >> int count; >> @@ -290,7 +291,8 @@ int pstore_mkfile(enum pstore_type_id type, char *psname, u64 id, int count, >> list_for_each_entry(pos, &allpstore, list) { >> if (pos->type == type && >> pos->id == id && >> - pos->psi == psi) { >> + pos->psi == psi && >> + !timespec_compare(&pos->time, &time)) { >> rc = -EEXIST; >> break; >> } >> @@ -312,6 +314,7 @@ int pstore_mkfile(enum pstore_type_id type, char *psname, u64 id, int count, >> private->id = id; >> private->count = count; >> private->psi = psi; >> + memcpy(&private->time, &time, sizeof(struct timespec)); >> >> switch (type) { >> case PSTORE_TYPE_DMESG: > > > As discussed on IRC, why don't you compare the variable names? Howdy Richard, Let's analyze the name of "duplicate" entries: >> dump-type0-9-1-1382511508-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0 >> dump-type0-9-1-1382515661-C-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0 The have the same name except timestamp. In this case, there is no difference between compare names and compare timestamps. ( Yeah, I'm pretty sure the guid will be the same one -- LINUX_EFI_CRASH_GUID ) But efi is only one backend. For other backends, we don't sure if they will add timestamp to name. So... -- Best, Madper Xie. ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <87r4bcdok1.fsf-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH] pstore: avoid incorrectly mark entry as duplicate 2013-10-23 14:55 ` Madper Xie @ 2013-10-23 21:57 ` Tony Luck -1 siblings, 0 replies; 10+ messages in thread From: Tony Luck @ 2013-10-23 21:57 UTC (permalink / raw) To: Madper Xie Cc: Anton Vorontsov, linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Seiji Aguchi, Lenny Szubowicz, 成骏 谢 On Wed, Oct 23, 2013 at 7:55 AM, Madper Xie <cxie-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote: > The "duplicate" entries won't appear in pstorefs. And a complain will be > print -- pstore: failed to load 76 record(s) from 'efi' Maybe I don't quite get this - but it sounds like you have a whole lot of entries using up space in efivars that have similar names - differing just in the timestamp - that won't show up in the pstore filesystem - because we'd try to name them all the same thing. How did all those things end up in efivars? Wouldn't the right fix be to make pstore allow them all to appear (using the timestamp to differentiate names?) so that we could see them, log them, and then remove them from pstore (in turn freeing up efivars space - which people keep telling me is in short supply). -Tony ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] pstore: avoid incorrectly mark entry as duplicate @ 2013-10-23 21:57 ` Tony Luck 0 siblings, 0 replies; 10+ messages in thread From: Tony Luck @ 2013-10-23 21:57 UTC (permalink / raw) To: Madper Xie Cc: Anton Vorontsov, linux-efi@vger.kernel.org, linux-kernel@vger.kernel.org, Seiji Aguchi, Lenny Szubowicz, 成骏 谢 On Wed, Oct 23, 2013 at 7:55 AM, Madper Xie <cxie@redhat.com> wrote: > The "duplicate" entries won't appear in pstorefs. And a complain will be > print -- pstore: failed to load 76 record(s) from 'efi' Maybe I don't quite get this - but it sounds like you have a whole lot of entries using up space in efivars that have similar names - differing just in the timestamp - that won't show up in the pstore filesystem - because we'd try to name them all the same thing. How did all those things end up in efivars? Wouldn't the right fix be to make pstore allow them all to appear (using the timestamp to differentiate names?) so that we could see them, log them, and then remove them from pstore (in turn freeing up efivars space - which people keep telling me is in short supply). -Tony ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] pstore: avoid incorrectly mark entry as duplicate 2013-10-23 21:57 ` Tony Luck (?) @ 2013-10-24 0:21 ` Madper Xie [not found] ` <87k3h35xij.fsf-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> -1 siblings, 1 reply; 10+ messages in thread From: Madper Xie @ 2013-10-24 0:21 UTC (permalink / raw) To: Tony Luck Cc: Madper Xie, Anton Vorontsov, linux-efi@vger.kernel.org, linux-kernel@vger.kernel.org, Seiji Aguchi, Lenny Szubowicz, 成骏 谢 tony.luck@gmail.com writes: > On Wed, Oct 23, 2013 at 7:55 AM, Madper Xie <cxie@redhat.com> wrote: >> The "duplicate" entries won't appear in pstorefs. And a complain will be >> print -- pstore: failed to load 76 record(s) from 'efi' > > Maybe I don't quite get this - but it sounds like you have a whole lot > of entries using up space in efivars that have similar names - differing > just in the timestamp - that won't show up in the pstore filesystem - because > we'd try to name them all the same thing. > Maybe I misunderstand you... Sure pstore try to name them all the same thing, but it's another issue. and it doesn't prevent entries showing up in pstore fs. Consider the following case: (after efi-pstore support append mode, it always like this case): I choose four dumped efivars from my DELL XPS: dump-type0-9-1-1380441690-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0 dump-type0-9-1-1380448560-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0 dump-type0-9-1-1380460890-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0 dump-type0-9-1-1382496073-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0 ^ ^ [ ^ ] ! ! ! type timestamp GUID When pstore load them from efivars, pstore incorrectly assuming that efivars with the same TYPE, ID and GUID are duplicate. list_for_each_entry(pos, &allpstore, list) { if (pos->type == type && /--- pos->id == id && <-| as I said, it only check type,id,psi pos->psi == psi) { \--- rc = -EEXIST; <- then set -EEXIST, and ignore *dup* entry break; } } You can see the code above, for those four entries, only one could be showed in pstorefs, all others will get a -EEXIST. So I add a timestamp check here, it's the only different part. > How did all those things end up in efivars? before the patch I can see dmesg-efi-1 dmesg-efi-11 dmesg-efi-3 dmesg-efi-5 dmesg-efi-7 dmesg-efi-9 dmesg-efi-10 dmesg-efi-2 dmesg-efi-4 dmesg-efi-6 dmesg-efi-8 after apply the patch: [root@dhcp-13-41 vars]# ls /dev/pstore/ dmesg-efi-1 dmesg-efi-1 dmesg-efi-11 dmesg-efi-2 dmesg-efi-3 dmesg-efi-5 dmesg-efi-6 dmesg-efi-8 dmesg-efi-1 dmesg-efi-10 dmesg-efi-11 dmesg-efi-2 dmesg-efi-3 dmesg-efi-5 dmesg-efi-7 dmesg-efi-8 dmesg-efi-1 dmesg-efi-10 dmesg-efi-12 dmesg-efi-2 dmesg-efi-4 dmesg-efi-5 dmesg-efi-7 dmesg-efi-8 dmesg-efi-1 dmesg-efi-10 dmesg-efi-13 dmesg-efi-2 dmesg-efi-4 dmesg-efi-5 dmesg-efi-7 dmesg-efi-9 dmesg-efi-1 dmesg-efi-10 dmesg-efi-14 dmesg-efi-2 dmesg-efi-4 dmesg-efi-5 dmesg-efi-7 dmesg-efi-9 dmesg-efi-1 dmesg-efi-10 dmesg-efi-15 dmesg-efi-2 dmesg-efi-4 dmesg-efi-5 dmesg-efi-7 dmesg-efi-9 dmesg-efi-1 dmesg-efi-10 dmesg-efi-16 dmesg-efi-3 dmesg-efi-4 dmesg-efi-5 dmesg-efi-7 dmesg-efi-9 dmesg-efi-1 dmesg-efi-10 dmesg-efi-2 dmesg-efi-3 dmesg-efi-4 dmesg-efi-6 dmesg-efi-7 dmesg-efi-9 dmesg-efi-1 dmesg-efi-10 dmesg-efi-2 dmesg-efi-3 dmesg-efi-4 dmesg-efi-6 dmesg-efi-7 dmesg-efi-9 dmesg-efi-1 dmesg-efi-10 dmesg-efi-2 dmesg-efi-3 dmesg-efi-4 dmesg-efi-6 dmesg-efi-8 dmesg-efi-9 dmesg-efi-1 dmesg-efi-11 dmesg-efi-2 dmesg-efi-3 dmesg-efi-4 dmesg-efi-6 dmesg-efi-8 dmesg-efi-9 dmesg-efi-1 dmesg-efi-11 dmesg-efi-2 dmesg-efi-3 dmesg-efi-4 dmesg-efi-6 dmesg-efi-8 dmesg-efi-9 dmesg-efi-1 dmesg-efi-11 dmesg-efi-2 dmesg-efi-3 dmesg-efi-5 dmesg-efi-6 dmesg-efi-8 dmesg-efi-1 dmesg-efi-11 dmesg-efi-2 dmesg-efi-3 dmesg-efi-5 dmesg-efi-6 dmesg-efi-8 dmesg-efi-1 dmesg-efi-11 dmesg-efi-2 dmesg-efi-3 dmesg-efi-5 dmesg-efi-6 dmesg-efi-8 > > Wouldn't the right fix be to make pstore allow them all to appear (using the > timestamp to differentiate names?) so that we could see them, log them, > and then remove them from pstore (in turn freeing up efivars space - which > people keep telling me is in short supply). > Yeah, many file have the same name, just like my case above. But it not really block the file shows up and should be solved in another patch. And I'm trying fix it. > -Tony -- Best, Madper Xie. ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <87k3h35xij.fsf-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH] pstore: avoid incorrectly mark entry as duplicate 2013-10-24 0:21 ` Madper Xie @ 2013-10-28 13:36 ` Madper Xie 0 siblings, 0 replies; 10+ messages in thread From: Madper Xie @ 2013-10-28 13:36 UTC (permalink / raw) To: Tony Luck Cc: Anton Vorontsov, linux-efi@vger.kernel.org, linux-kernel@vger.kernel.org, Seiji Aguchi, Lenny Szubowicz, 成骏 谢, Colin Cross, Kees Cook Howdy Tony, Does this patch still need some rework? cxie-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org writes: > tony.luck-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org writes: > >> On Wed, Oct 23, 2013 at 7:55 AM, Madper Xie <cxie-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote: >>> The "duplicate" entries won't appear in pstorefs. And a complain will be >>> print -- pstore: failed to load 76 record(s) from 'efi' >> >> Maybe I don't quite get this - but it sounds like you have a whole lot >> of entries using up space in efivars that have similar names - differing >> just in the timestamp - that won't show up in the pstore filesystem - because >> we'd try to name them all the same thing. >> > Maybe I misunderstand you... > > Sure pstore try to name them all the same thing, but it's another > issue. and it doesn't prevent entries showing up in pstore fs. > > Consider the following case: (after efi-pstore support append mode, it > always like this case): > > I choose four dumped efivars from my DELL XPS: > dump-type0-9-1-1380441690-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0 > dump-type0-9-1-1380448560-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0 > dump-type0-9-1-1380460890-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0 > dump-type0-9-1-1382496073-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0 > ^ ^ [ ^ ] > ! ! ! > type timestamp GUID > > When pstore load them from efivars, pstore incorrectly assuming that > efivars with the same TYPE, ID and GUID are duplicate. > > list_for_each_entry(pos, &allpstore, list) { > if (pos->type == type && /--- > pos->id == id && <-| as I said, it only check type,id,psi > pos->psi == psi) { \--- > rc = -EEXIST; <- then set -EEXIST, and ignore *dup* entry > break; > } > } > You can see the code above, for those four entries, only one could be > showed in pstorefs, all others will get a -EEXIST. So I add a timestamp > check here, it's the only different part. > >> How did all those things end up in efivars? > before the patch I can see > dmesg-efi-1 dmesg-efi-11 dmesg-efi-3 dmesg-efi-5 dmesg-efi-7 dmesg-efi-9 > dmesg-efi-10 dmesg-efi-2 dmesg-efi-4 dmesg-efi-6 dmesg-efi-8 > after apply the patch: > [root@dhcp-13-41 vars]# ls /dev/pstore/ > dmesg-efi-1 dmesg-efi-1 dmesg-efi-11 dmesg-efi-2 dmesg-efi-3 dmesg-efi-5 dmesg-efi-6 dmesg-efi-8 > dmesg-efi-1 dmesg-efi-10 dmesg-efi-11 dmesg-efi-2 dmesg-efi-3 dmesg-efi-5 dmesg-efi-7 dmesg-efi-8 > dmesg-efi-1 dmesg-efi-10 dmesg-efi-12 dmesg-efi-2 dmesg-efi-4 dmesg-efi-5 dmesg-efi-7 dmesg-efi-8 > dmesg-efi-1 dmesg-efi-10 dmesg-efi-13 dmesg-efi-2 dmesg-efi-4 dmesg-efi-5 dmesg-efi-7 dmesg-efi-9 > dmesg-efi-1 dmesg-efi-10 dmesg-efi-14 dmesg-efi-2 dmesg-efi-4 dmesg-efi-5 dmesg-efi-7 dmesg-efi-9 > dmesg-efi-1 dmesg-efi-10 dmesg-efi-15 dmesg-efi-2 dmesg-efi-4 dmesg-efi-5 dmesg-efi-7 dmesg-efi-9 > dmesg-efi-1 dmesg-efi-10 dmesg-efi-16 dmesg-efi-3 dmesg-efi-4 dmesg-efi-5 dmesg-efi-7 dmesg-efi-9 > dmesg-efi-1 dmesg-efi-10 dmesg-efi-2 dmesg-efi-3 dmesg-efi-4 dmesg-efi-6 dmesg-efi-7 dmesg-efi-9 > dmesg-efi-1 dmesg-efi-10 dmesg-efi-2 dmesg-efi-3 dmesg-efi-4 dmesg-efi-6 dmesg-efi-7 dmesg-efi-9 > dmesg-efi-1 dmesg-efi-10 dmesg-efi-2 dmesg-efi-3 dmesg-efi-4 dmesg-efi-6 dmesg-efi-8 dmesg-efi-9 > dmesg-efi-1 dmesg-efi-11 dmesg-efi-2 dmesg-efi-3 dmesg-efi-4 dmesg-efi-6 dmesg-efi-8 dmesg-efi-9 > dmesg-efi-1 dmesg-efi-11 dmesg-efi-2 dmesg-efi-3 dmesg-efi-4 dmesg-efi-6 dmesg-efi-8 dmesg-efi-9 > dmesg-efi-1 dmesg-efi-11 dmesg-efi-2 dmesg-efi-3 dmesg-efi-5 dmesg-efi-6 dmesg-efi-8 > dmesg-efi-1 dmesg-efi-11 dmesg-efi-2 dmesg-efi-3 dmesg-efi-5 dmesg-efi-6 dmesg-efi-8 > dmesg-efi-1 dmesg-efi-11 dmesg-efi-2 dmesg-efi-3 dmesg-efi-5 dmesg-efi-6 dmesg-efi-8 > >> >> Wouldn't the right fix be to make pstore allow them all to appear (using the >> timestamp to differentiate names?) so that we could see them, log them, >> and then remove them from pstore (in turn freeing up efivars space - which >> people keep telling me is in short supply). >> > Yeah, many file have the same name, just like my case above. But it not > really block the file shows up and should be solved in another > patch. And I'm trying fix it. >> -Tony -- Best, Madper Xie. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] pstore: avoid incorrectly mark entry as duplicate @ 2013-10-28 13:36 ` Madper Xie 0 siblings, 0 replies; 10+ messages in thread From: Madper Xie @ 2013-10-28 13:36 UTC (permalink / raw) To: Tony Luck Cc: Anton Vorontsov, linux-efi@vger.kernel.org, linux-kernel@vger.kernel.org, Seiji Aguchi, Lenny Szubowicz, 成骏 谢, Colin Cross, Kees Cook Howdy Tony, Does this patch still need some rework? cxie@redhat.com writes: > tony.luck@gmail.com writes: > >> On Wed, Oct 23, 2013 at 7:55 AM, Madper Xie <cxie@redhat.com> wrote: >>> The "duplicate" entries won't appear in pstorefs. And a complain will be >>> print -- pstore: failed to load 76 record(s) from 'efi' >> >> Maybe I don't quite get this - but it sounds like you have a whole lot >> of entries using up space in efivars that have similar names - differing >> just in the timestamp - that won't show up in the pstore filesystem - because >> we'd try to name them all the same thing. >> > Maybe I misunderstand you... > > Sure pstore try to name them all the same thing, but it's another > issue. and it doesn't prevent entries showing up in pstore fs. > > Consider the following case: (after efi-pstore support append mode, it > always like this case): > > I choose four dumped efivars from my DELL XPS: > dump-type0-9-1-1380441690-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0 > dump-type0-9-1-1380448560-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0 > dump-type0-9-1-1380460890-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0 > dump-type0-9-1-1382496073-cfc8fc79-be2e-4ddc-97f0-9f98bfe298a0 > ^ ^ [ ^ ] > ! ! ! > type timestamp GUID > > When pstore load them from efivars, pstore incorrectly assuming that > efivars with the same TYPE, ID and GUID are duplicate. > > list_for_each_entry(pos, &allpstore, list) { > if (pos->type == type && /--- > pos->id == id && <-| as I said, it only check type,id,psi > pos->psi == psi) { \--- > rc = -EEXIST; <- then set -EEXIST, and ignore *dup* entry > break; > } > } > You can see the code above, for those four entries, only one could be > showed in pstorefs, all others will get a -EEXIST. So I add a timestamp > check here, it's the only different part. > >> How did all those things end up in efivars? > before the patch I can see > dmesg-efi-1 dmesg-efi-11 dmesg-efi-3 dmesg-efi-5 dmesg-efi-7 dmesg-efi-9 > dmesg-efi-10 dmesg-efi-2 dmesg-efi-4 dmesg-efi-6 dmesg-efi-8 > after apply the patch: > [root@dhcp-13-41 vars]# ls /dev/pstore/ > dmesg-efi-1 dmesg-efi-1 dmesg-efi-11 dmesg-efi-2 dmesg-efi-3 dmesg-efi-5 dmesg-efi-6 dmesg-efi-8 > dmesg-efi-1 dmesg-efi-10 dmesg-efi-11 dmesg-efi-2 dmesg-efi-3 dmesg-efi-5 dmesg-efi-7 dmesg-efi-8 > dmesg-efi-1 dmesg-efi-10 dmesg-efi-12 dmesg-efi-2 dmesg-efi-4 dmesg-efi-5 dmesg-efi-7 dmesg-efi-8 > dmesg-efi-1 dmesg-efi-10 dmesg-efi-13 dmesg-efi-2 dmesg-efi-4 dmesg-efi-5 dmesg-efi-7 dmesg-efi-9 > dmesg-efi-1 dmesg-efi-10 dmesg-efi-14 dmesg-efi-2 dmesg-efi-4 dmesg-efi-5 dmesg-efi-7 dmesg-efi-9 > dmesg-efi-1 dmesg-efi-10 dmesg-efi-15 dmesg-efi-2 dmesg-efi-4 dmesg-efi-5 dmesg-efi-7 dmesg-efi-9 > dmesg-efi-1 dmesg-efi-10 dmesg-efi-16 dmesg-efi-3 dmesg-efi-4 dmesg-efi-5 dmesg-efi-7 dmesg-efi-9 > dmesg-efi-1 dmesg-efi-10 dmesg-efi-2 dmesg-efi-3 dmesg-efi-4 dmesg-efi-6 dmesg-efi-7 dmesg-efi-9 > dmesg-efi-1 dmesg-efi-10 dmesg-efi-2 dmesg-efi-3 dmesg-efi-4 dmesg-efi-6 dmesg-efi-7 dmesg-efi-9 > dmesg-efi-1 dmesg-efi-10 dmesg-efi-2 dmesg-efi-3 dmesg-efi-4 dmesg-efi-6 dmesg-efi-8 dmesg-efi-9 > dmesg-efi-1 dmesg-efi-11 dmesg-efi-2 dmesg-efi-3 dmesg-efi-4 dmesg-efi-6 dmesg-efi-8 dmesg-efi-9 > dmesg-efi-1 dmesg-efi-11 dmesg-efi-2 dmesg-efi-3 dmesg-efi-4 dmesg-efi-6 dmesg-efi-8 dmesg-efi-9 > dmesg-efi-1 dmesg-efi-11 dmesg-efi-2 dmesg-efi-3 dmesg-efi-5 dmesg-efi-6 dmesg-efi-8 > dmesg-efi-1 dmesg-efi-11 dmesg-efi-2 dmesg-efi-3 dmesg-efi-5 dmesg-efi-6 dmesg-efi-8 > dmesg-efi-1 dmesg-efi-11 dmesg-efi-2 dmesg-efi-3 dmesg-efi-5 dmesg-efi-6 dmesg-efi-8 > >> >> Wouldn't the right fix be to make pstore allow them all to appear (using the >> timestamp to differentiate names?) so that we could see them, log them, >> and then remove them from pstore (in turn freeing up efivars space - which >> people keep telling me is in short supply). >> > Yeah, many file have the same name, just like my case above. But it not > really block the file shows up and should be solved in another > patch. And I'm trying fix it. >> -Tony -- Best, Madper Xie. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2013-10-28 13:37 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-23 14:55 [PATCH] pstore: avoid incorrectly mark entry as duplicate Madper Xie
2013-10-23 14:55 ` Madper Xie
2013-10-23 15:58 ` Richard Weinberger
[not found] ` <CAFLxGvxT1S3km=LordqFGbAb_pWc0cf3oE2ciS9uY4BBJJ231A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-10-23 16:11 ` Madper Xie
2013-10-23 16:11 ` Madper Xie
[not found] ` <87r4bcdok1.fsf-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2013-10-23 21:57 ` Tony Luck
2013-10-23 21:57 ` Tony Luck
2013-10-24 0:21 ` Madper Xie
[not found] ` <87k3h35xij.fsf-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2013-10-28 13:36 ` Madper Xie
2013-10-28 13:36 ` Madper Xie
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.