On Fri, 25 Oct 2024 18:11:01 +0900,
Johannes Berg wrote:
>
> (I should say, I'm still reading through this, and haven't formed an
> overall opinion. Just nitpicking on the details as I see them for now)
thanks anyway. looking forward to any opinions.
> > +#endif
> > +
> >
> > #include <asm-generic/mmu_context.h>
>
> extra newline
will fix it.
> > /* tlb.c */
> > +#ifdef CONFIG_MMU
> > extern void report_enomem(void);
> > +#else
> > +static inline void report_enomem(void)
> > +{
> > +}
> > +#endif
>
> Should that really do _nothing_? Perhaps it's not called at all in no-
> MMU, but then you don't need it, but otherwise it seems it should do
> something even if it's just panic()?
it is called also in !MMU. I'll think to figure out how the function
is shared.
>
> > brk_end = (unsigned long) UML_ROUND_UP(sbrk(0));
> > +#ifdef CONFIG_MMU
> > map_memory(brk_end, __pa(brk_end), uml_reserved - brk_end, 1, 1, 0);
> > +#else
> > + map_memory(brk_end, __pa(brk_end), uml_reserved - brk_end, 1, 1, 1);
> > +#endif
>
> That seems much simpler as
>
> map_memory(.....,
> !IS_ENABLED(CONFIG_MMU));
looks nice, will fix it.
> > +#ifdef UML_CONFIG_MMU
> > loc = mmap64((void *) virt, len, prot, MAP_SHARED | MAP_FIXED,
> > fd, off);
> > +#else
> > + loc = mmap64((void *) virt, len, prot, MAP_SHARED | MAP_FIXED |
> > MAP_ANONYMOUS,
> > + fd, off);
> > +#endif
>
> Same here,
>
> mmap64(....
> MAP_SHARED | MAP_FIXED |
> IS_ENABLED(CONFIG_MMU) ? MAP_ANONYMOUS : 0,
> ...);
since this is part under os-Linux and we cannot use kconfig.h (IIUC)
feature (e.g., IS_ENABLED). but I'll reformat it to simplify instead
of duplicating same lines.
-- Hajime
