On 11/7/23 5:02 PM, Mina Almasry wrote:
On Mon, Nov 6, 2023 at 1:02 PM Stanislav Fomichev sdf@google.com wrote:
On 11/05, Mina Almasry wrote:
+static inline bool page_is_page_pool_iov(const struct page *page) +{
return (unsigned long)page & PP_DEVMEM;
+}
Speaking of bpf: one thing that might be problematic with this PP_DEVMEM bit is that it will make debugging with bpftrace a bit (more) complicated. If somebody were trying to get to that page_pool_iov from the frags, they will have to do the equivalent of page_is_page_pool_iov, but probably not a big deal? (thinking out loud)
Good point, but that doesn't only apply to bpf I think. I'm guessing even debugger drgn access to the bv_page in the frag will have trouble if it's actually accessing an iov with LSB set.
But this is not specific to this use for LSB pointer trick. I think all code that currently uses LSB pointer trick will have similar troubles. In this context my humble vote is that we get such big upside from reducing code churn that it's reasonable to tolerate such side effects.
+1
I could alleviate some of the issues by teaching drgn to do the right thing for devmem/iovs... time permitting.
Tools like drgn and crash have to know when the LSB trick is used - e.g., dst_entry - and handle it when dereferencing pointers.