On 5/2/19 3:51 AM, Sebastian Andrzej Siewior wrote:

On 2019-05-01 14:29:17 [-0700], John Johansen wrote:

quoted

On 4/30/19 7:47 AM, Sebastian Andrzej Siewior wrote:

quoted

On 2019-04-28 16:56:59 [-0700], John Johansen wrote:

quoted

So digging into why the history of the per cpu buffers in apparmor.
We used to do buffer allocations via kmalloc and there were a few reasons
for the switch 

* speed/lockless: speaks for it self, mediation is already slow enough

it is shared among all CPUs but it is a small/quick operation to
add/return a buffer.

I wouldn't exactly call taking a lock speedy. Getting an available buffer
or returning it is indeed quick. The allocation fall back not so much.

Based on testing it happens only in the beginning. We could also start
with 2,3,4 pre allocated buffers or so.
My testing was most likely limited and I did not exceed two.

yeah lets have a few preallocated

quoted

quoted

quoted

* some buffer allocations had to be done with GFP_ATOMIC, making them
  more likely to fail. Since we fail closed that means failure would
  block access. This actually became a serious problem in a couple
  places. Switching to per cpu buffers and blocking pre-empt was
  the solution.

GFP_KERNEL is allowed to use IO/SWAP and ATOMIC has emergency pools. The
new approach won't return a NULL pointer, simply spin to either allocate
new memory or get one which was just returned.

yeah, I am not really a fan of a potential infinite loop trying to allocate
memory. It may be worth retrying once or twice but potentially infinitely
spinning on failed allocation really isn't acceptable.

It shouldn't spin infinitely because even if kmalloc() does not return
any memory, one of the other CPUs should return their buffer at some
point. However, if you don't like it I could add two retries and return
NULL + fixup callers. On the other hand if the other CPUs BUG() with the
buffers then yes, we may spin.
So limited retries it is?

yes please