On Mon, Jan 18, 2021 at 09:24:17AM +0000, John Garry wrote: > On 15/01/2021 17:30, Jean-Philippe Brucker wrote: > > On Thu, Dec 10, 2020 at 02:23:08AM +0800, John Garry wrote: > > > A similar crash to the following could be observed if initial CPU rcache > > > magazine allocations fail in init_iova_rcaches(): > > > > thanks for having a look > > > Any idea why that's happening? This fix seems ok but if we're expecting > > allocation failures for the loaded magazine then we could easily get it > > for cpu_rcaches too, and get a similar abort at runtime. > > It's not specifically that we expect them (allocation failures for the > loaded magazine), rather we should make safe against it. > > So could you be more specific in your concern for the cpu_rcache failure? > cpu_rcache magazine assignment comes from this logic.
If this fails: drivers/iommu/iova.c:847: rcache->cpu_rcaches = __alloc_percpu(sizeof(*cpu_rcache), cache_line_size()); then we'll get an Oops in __iova_rcache_get(). So if we're making the module safer against magazine allocation failure, shouldn't we also protect against cpu_rcaches allocation failure? Thanks, Jean > > Anyway, logic like "if not full" or "if not empty" is poor as the outcome > for NULL is ambiguous (maybe there's a better word) and the code is not safe > against it, and so I replace with "if space" or "if have an IOVA", > respectively. > > Thanks, > John >