| Message ID | YhIq94B0MpYGrEm2@zn.tnic (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [GIT,PULL] EDAC fix for 5.17 | expand |
On Sun, Feb 20, 2022 at 3:50 AM Borislav Petkov <bp@suse.de> wrote: > > please pull a fix for a long-standing, hard-to-catch issue in the EDAC > weird struct allocation code, for 5.17. Heh. Yeah, that was subtly buggy. Side note: the comment says that it will align to at least as much as the compiler would do, but then it does something different, eg if (size > sizeof(long)) align = sizeof(long long); ... and it might make sense to use "__alignof__()" instead of "sizeof()" just to make the code match the comment. For example, on 32-bit x86, "sizeof(long long)" is 8, but "__alignof__(long long)" is only 4. And then we have m68k.. Or maybe the comment should be fixed instead, and talk about "natural alignment" rather than "compiler alignment". Linus
The pull request you sent on Sun, 20 Feb 2022 12:50:15 +0100:
> git://git.kernel.org/pub/scm/linux/kernel/git/ras/ras.git tags/edac_urgent_for_v5.17_rc5
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/6e8e752f705c2713005a3182c8444ef7b54f10aa
Thank you!
On Sun, Feb 20, 2022 at 12:12:41PM -0800, Linus Torvalds wrote: > Or maybe the comment should be fixed instead, and talk about "natural > alignment" rather than "compiler alignment". Yah, where do I start... so, about this, I think I can simplify it by simply unconditionally aligning to 8. My gut feeling is telling me 8-bytes alignment should simply work on everything. Because if it does, all that crap becomes a lot simpler. But maybe I'm being too simplistic here and there might be a corner-case where 8-bytes alignment just doesn't work... Then, that edac_align_ptr() thing is an abomination. It probably has made sense at some point to allocate the whole structure, including the embedded pointers in one go but I can't recall of ever seeing something like that done somewhere else around the kernel. But maybe you'll know of another example and why that would have made sense in the past. If not, I'm thinking of gradually converting all drivers to do normal structs allocation like the rest of the tree does and then getting rid of that thing. And I keep hoping someone else would volunteer but no one has so far... Thx.
On Sun, Feb 20, 2022 at 12:25 PM Borislav Petkov <bp@suse.de> wrote: > > Yah, where do I start... so, about this, I think I can simplify it by > simply unconditionally aligning to 8. Sounds good. Then you could just do something like void *ptr = (void *)ALIGN_UP((unsigned long)*p, 8); *p = ptr + size*n_memb; return ptr; and that would be a lot simpler. > My gut feeling is telling me > 8-bytes alignment should simply work on everything. Because if it does, > all that crap becomes a lot simpler. But maybe I'm being too simplistic > here and there might be a corner-case where 8-bytes alignment just > doesn't work... Well, if 8-byte alignment doesn't work, then the existing code (with the fix) doesn't work either, so.. Linus
Hi Linus, please pull a fix for a long-standing, hard-to-catch issue in the EDAC weird struct allocation code, for 5.17. Thx. --- The following changes since commit 754e0b0e35608ed5206d6a67a791563c631cec07: Linux 5.17-rc4 (2022-02-13 12:13:30 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/ras/ras.git tags/edac_urgent_for_v5.17_rc5 for you to fetch changes up to f8efca92ae509c25e0a4bd5d0a86decea4f0c41e: EDAC: Fix calculation of returned address and next offset in edac_align_ptr() (2022-02-15 15:54:46 +0100) ---------------------------------------------------------------- - Fix a long-standing struct alignment bug in the EDAC struct allocation code ---------------------------------------------------------------- Eliav Farber (1): EDAC: Fix calculation of returned address and next offset in edac_align_ptr() drivers/edac/edac_mc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)