Messages in this thread Patch in this message | | | Date | Fri, 16 Nov 2012 18:53:55 -0800 | From | Anton Vorontsov <> | Subject | Re: [PATCH v2] pstore/ram: no timekeeping calls when unavailable |
| |
On Fri, Nov 09, 2012 at 05:26:53PM -0800, Kees Cook wrote: [....] > >> @@ -171,7 +171,13 @@ static size_t ramoops_write_kmsg_hdr(struct > >> persistent_ram_zone *prz) > >> struct timeval timestamp; > >> size_t len; > >> > >> - do_gettimeofday(×tamp); > >> + /* Handle dumping before timekeeping has resumed. */ > >> + if (unlikely(timekeeping_suspended)) { > >> + timestamp.tv_sec = 0; > >> + timestamp.tv_usec = 0; > >> + } else > >> + do_gettimeofday(×tamp); > >> + > > > > Would nulling out the timestamp be better done in do_gettimeofday()? That > > way we don't have to export timekeeping internals and users would get > > something more sane for this corner case. > > Well... I'm not sure. If we don't want to expose the > timekeeping_suspended variable, maybe we need a function to check > this? I think it's probably better to find the users of timekeeping > that could call it when suspended. That's why I figured the BUG was > there. Very very few things should be attempting to call gettimeofday > in a place where it might be suspended. As such, it seems like those > things should be able to determine how to handle it. Maybe not > everything would be sensible to get back 0s. > > In this particular case, I'm fine with removing the BUG and returning > 0 instead, since that's fine for ramoops. :)
In the lack of agreement on kernel/time/timekeeping.c change, I can't apply the patch. And personally I tend to agree that doing this workaround in the pstore code is odd. How about introducing ___do_gettimeofday() that is safe to call when suspened, and the func would have good kernel doc comments explaining the purpose of it?
Thanks, Anton.
| |