the chance to block another snapshot creation is tiny. but block the map() is possible. i do not see anything that makes this patch not as good as the original one. ming On Fri, 2004-11-19 at 15:51, Kevin Corry wrote: > On Friday 19 November 2004 2:27 pm, Alasdair G Kergon wrote: > > On Fri, Nov 19, 2004 at 02:16:52PM -0600, Kevin Corry wrote: > > > In register_snapshot(), move the kmalloc() outside the _origins_lock. > > > > Maybe - but snapshots and memory allocations don't mix particularly well, > > and these are complex trade-offs, so for now I'd rather leave this code > > as it is (i.e. not introduce new allocations), until there's strong > > evidence one way or the other e.g. from testing or more detailed code > > analysis. > > I wasn't implying you needed to (or even should) accept that patch. :) Just > put it out there as an example of how to reduce the time spent holding that > lock without having to drop and reaquire it. I guess I should have marked my > response as [RFC|PATCH]. > > Personally, I think this is one area where we really don't have to worry about > lock contention. It's very unlikely in real-world scenarios that two > different processes are going to be creating snapshots at the same time. > Thinking about EVMS, I know there's no way to activate two snapshots > simultaneously. Thinking about LVM2, you'd have to run two copies of lvcreate > at the same point in time, which also seems kind of hard to imagine in > practice. > > Thus, I'd say the patch is mostly unnecessary.