Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756595Ab3J1NhS (ORCPT ); Mon, 28 Oct 2013 09:37:18 -0400 Received: from mx1.redhat.com ([209.132.183.28]:63284 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755303Ab3J1NhP (ORCPT ); Mon, 28 Oct 2013 09:37:15 -0400 References: <87r4bcdok1.fsf@redhat.com> <87k3h35xij.fsf@redhat.com> User-agent: mu4e 0.9.9.5; emacs 24.3.1 From: Madper Xie To: Tony Luck Cc: Anton Vorontsov , "linux-efi\@vger.kernel.org" , "linux-kernel\@vger.kernel.org" , Seiji Aguchi , Lenny Szubowicz , =?utf-8?B?5oiQ6aqPIOiwog==?= , Colin Cross , Kees Cook Subject: Re: [PATCH] pstore: avoid incorrectly mark entry as duplicate In-reply-to: <87k3h35xij.fsf@redhat.com> Date: Mon, 28 Oct 2013 21:36:57 +0800 Message-ID: <87iowhv7nq.fsf@redhat.com> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4533 Lines: 90 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 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/