{"payload":{"feedbackUrl":"https://github.com/orgs/community/discussions/53140","repo":{"id":390994078,"defaultBranch":"master","name":"linux","ownerLogin":"emlix","currentUserCanPush":false,"isFork":true,"isEmpty":false,"createdAt":"2021-07-30T08:43:15.000Z","ownerAvatar":"https://avatars.githubusercontent.com/u/51944257?v=4","public":true,"private":false,"isOrgOwned":true},"refInfo":{"name":"","listCacheKey":"v0:1632470199.0576482","currentOid":""},"activityList":{"items":[{"before":"f6feea56f66d34259c4222fa02e8171c4f2673d1","after":"63355b9884b3d1677de6bd1517cd2b8a9bf53978","ref":"refs/heads/master","pushedAt":"2023-03-08T08:11:26.307Z","pushType":"push","commitsCount":10000,"pusher":{"login":"DerDakon","name":"Rolf Eike Beer","path":"/DerDakon","primaryAvatarUrl":"https://avatars.githubusercontent.com/u/10909049?s=80&v=4"},"commit":{"message":"cpumask: be more careful with 'cpumask_setall()'\n\nCommit 596ff4a09b89 (\"cpumask: re-introduce constant-sized cpumask\noptimizations\") changed cpumask_setall() to use \"bitmap_set()\" instead\nof \"bitmap_fill()\", because bitmap_fill() would explicitly set all the\nbits of a constant sized small bitmap, and that's exactly what we don't\nwant: we want to only set bits up to 'nr_cpu_ids', which is what\n\"bitmap_set()\" does.\n\nHowever, Yury correctly points out that while \"bitmap_set()\" does indeed\nonly set bits up to the required bitmap size, it doesn't _clear_ bits\nabove that size, so the upper bits would still not have well-defined\nvalues.\n\nNow, none of this should really matter, since any bits set past\n'nr_cpu_ids' should always be ignored in the first place.  Yes, the bit\nscanning functions might return them as a result, but since users should\nalways consider the \">= nr_cpu_ids\" condition to mean \"no more bits\",\nthat shouldn't have any actual effect (see previous commit 8ca09d5fa354\n\"cpumask: fix incorrect cpumask scanning result checks\").\n\nBut let's just do it right, the way the code was _intended_ to work.  We\nhave had enough lazy code that works but bites us in the *rse later\n(again, see previous commit) that there's no reason to not just do this\nproperly.\n\nIt turns out that \"bitmap_fill()\" gets this all right for the complex\ncase, and really only fails for the inlined optimized case that just\nfills the whole word.  And while we could just fix bitmap_fill() to use\nthe proper last word mask, there's two issues with that:\n\n - the cpumask case wants to do the _optimization_ based on \"NR_CPUS is\n   a small constant\", but then wants to do the actual bit _fill_ based\n   on \"nr_cpu_ids\" that isn't necessarily that same constant\n\n - we have lots of non-cpumask users of bitmap_fill(), and while they\n   hopefully don't care, and probably would want the proper semantics\n   anyway (\"only set bits up to the limit\"), I do not want the cpumask\n   changes to impact other parts\n\nSo this ends up just doing the single-word optimization by hand in the\ncpumask code.  If our cpumask is fundamentally limited to a single word,\njust do the proper \"fill in that word\" exactly.  And if it's the more\ncomplex multi-word case, then the generic bitmap_fill() will DTRT.\n\nThis is all an example of how our bitmap function optimizations really\nare somewhat broken.  They conflate the \"this is size of the bitmap\"\noptimizations with the actual bit(s) we want to set.\n\nIn many cases we really want to have the two be separate things:\nsometimes we base our optimizations on the size of the whole bitmap (\"I\nknow this whole bitmap fits in a single word, so I'll just use\nsingle-word accesses\"), and sometimes we base them on the bit we are\nlooking at (\"this is just acting on bits that are in the first word, so\nI'll use single-word accesses\").\n\nNotice how the end result of the two optimizations are the same, but the\nway we get to them are quite different.\n\nAnd all our cpumask optimization games are really about that fundamental\ndistinction, and we'd often really want to pass in both the \"this is the\nbit I'm working on\" (which _can_ be a small constant but might be\nvariable), and \"I know it's in this range even if it's variable\" (based\non CONFIG_NR_CPUS).\n\nSo this cpumask_setall() implementation just makes that explicit.  It\nchecks the \"I statically know the size is small\" using the known static\nsize of the cpumask (which is what that 'small_cpumask_bits' is all\nabout), but then sets the actual bits using the exact number of cpus we\nhave (ie 'nr_cpumask_bits')\n\nOf course, in a perfect world, the compiler would have done all the\nrange analysis (possibly with help from us just telling it that\n\"this value is always in this range\"), and would do all of this for us.\nBut that is not the world we live in.\n\nWhile we dream of that perfect world, this does that manual logic to\nmake it all work out.  And this was a very long explanation for a small\ncode change that shouldn't even matter.\n\nReported-by: Yury Norov <yury.norov@gmail.com>\nLink: https://lore.kernel.org/lkml/ZAV9nGG9e1%2FrV+L%2F@yury-laptop/\nSigned-off-by: Linus Torvalds <torvalds@linux-foundation.org>","shortMessageHtmlLink":"cpumask: be more careful with 'cpumask_setall()'"}}],"hasNextPage":false,"hasPreviousPage":false,"activityType":"all","actor":null,"timePeriod":"all","sort":"DESC","perPage":30,"cursor":"djE6ks8AAAAC_tScdQA","startCursor":null,"endCursor":null}},"title":"Activity · emlix/linux"}