Remove the sanity_check() function, as it is used nowhere.

Since any application linked with Mini-OS can't call sanity_check()
either (there is no EXPORT_SYMBOL for it), there is zero chance of
breaking any use case.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 include/lib.h |  3 ---
 mm.c          | 16 ----------------
 2 files changed, 19 deletions(-)

diff --git a/include/lib.h b/include/lib.h
index abd4e9ab..acd4acc6 100644
--- a/include/lib.h
+++ b/include/lib.h
@@ -152,9 +152,6 @@ do {                                                           \
 
 #define BUG_ON(x) ASSERT(!(x))
 
-/* Consistency check as much as possible. */
-void sanity_check(void);
-
 /* Get own domid. */
 domid_t get_domid(void);
 
diff --git a/mm.c b/mm.c
index 96686a5c..1fa7e7bf 100644
--- a/mm.c
+++ b/mm.c
@@ -394,19 +394,3 @@ void init_mm(void)
 void fini_mm(void)
 {
 }
-
-void sanity_check(void)
-{
-    int x;
-    chunk_head_t *head;
-
-    for ( x = 0; x < FREELIST_SIZE; x++ )
-    {
-        for ( head = free_list[x].next; !FREELIST_EMPTY(head);
-              head = head->next )
-        {
-            ASSERT(!allocated_in_map(virt_to_pfn(head)));
-            ASSERT(head->next->prev == head);
-        }
-    }
-}
-- 
2.43.0

Juergen Gross, le lun. 22 juil. 2024 17:01:41 +0200, a ecrit:
> Remove the sanity_check() function, as it is used nowhere.
> 
> Since any application linked with Mini-OS can't call sanity_check()
> either (there is no EXPORT_SYMBOL for it), there is zero chance of
> breaking any use case.

Don't we still want to keep it around, at least as formal documentation
of the expected status of the list?

Samuel

> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  include/lib.h |  3 ---
>  mm.c          | 16 ----------------
>  2 files changed, 19 deletions(-)
> 
> diff --git a/include/lib.h b/include/lib.h
> index abd4e9ab..acd4acc6 100644
> --- a/include/lib.h
> +++ b/include/lib.h
> @@ -152,9 +152,6 @@ do {                                                           \
>  
>  #define BUG_ON(x) ASSERT(!(x))
>  
> -/* Consistency check as much as possible. */
> -void sanity_check(void);
> -
>  /* Get own domid. */
>  domid_t get_domid(void);
>  
> diff --git a/mm.c b/mm.c
> index 96686a5c..1fa7e7bf 100644
> --- a/mm.c
> +++ b/mm.c
> @@ -394,19 +394,3 @@ void init_mm(void)
>  void fini_mm(void)
>  {
>  }
> -
> -void sanity_check(void)
> -{
> -    int x;
> -    chunk_head_t *head;
> -
> -    for ( x = 0; x < FREELIST_SIZE; x++ )
> -    {
> -        for ( head = free_list[x].next; !FREELIST_EMPTY(head);
> -              head = head->next )
> -        {
> -            ASSERT(!allocated_in_map(virt_to_pfn(head)));
> -            ASSERT(head->next->prev == head);
> -        }
> -    }
> -}
> -- 
> 2.43.0
> 

-- 
Samuel
/*
 * [...] Note that 120 sec is defined in the protocol as the maximum
 * possible RTT.  I guess we'll have to use something other than TCP
 * to talk to the University of Mars.
 * PAWS allows us longer timeouts and large windows, so once implemented
 * ftp to mars will work nicely.
 */
(from /usr/src/linux/net/inet/tcp.c, concerning RTT [retransmission timeout])

On 22.07.24 23:35, Samuel Thibault wrote:
> Juergen Gross, le lun. 22 juil. 2024 17:01:41 +0200, a ecrit:
>> Remove the sanity_check() function, as it is used nowhere.
>>
>> Since any application linked with Mini-OS can't call sanity_check()
>> either (there is no EXPORT_SYMBOL for it), there is zero chance of
>> breaking any use case.
> 
> Don't we still want to keep it around, at least as formal documentation
> of the expected status of the list?

Hmm, is it really worth the extra code?

There are 2 ASSERT()s I'm deleting: one testing the allocation bitmap
to match the comment further up:

/*
  * ALLOCATION BITMAP
  *  One bit per page of memory. Bit set => page is allocated.
  */

And the other one testing the linked lists being correct, which IMO
doesn't need any further documentation.


Juergen

Hello,

Jürgen Groß, le mar. 23 juil. 2024 08:36:13 +0200, a ecrit:
> On 22.07.24 23:35, Samuel Thibault wrote:
> > Juergen Gross, le lun. 22 juil. 2024 17:01:41 +0200, a ecrit:
> > > Remove the sanity_check() function, as it is used nowhere.
> > > 
> > > Since any application linked with Mini-OS can't call sanity_check()
> > > either (there is no EXPORT_SYMBOL for it), there is zero chance of
> > > breaking any use case.
> > 
> > Don't we still want to keep it around, at least as formal documentation
> > of the expected status of the list?
> 
> Hmm, is it really worth the extra code?

I have already seen such kind of piece of code getting very convenient
when tracking odd bugs.

Samuel

On 25.07.24 00:44, Samuel Thibault wrote:
> Hello,
> 
> Jürgen Groß, le mar. 23 juil. 2024 08:36:13 +0200, a ecrit:
>> On 22.07.24 23:35, Samuel Thibault wrote:
>>> Juergen Gross, le lun. 22 juil. 2024 17:01:41 +0200, a ecrit:
>>>> Remove the sanity_check() function, as it is used nowhere.
>>>>
>>>> Since any application linked with Mini-OS can't call sanity_check()
>>>> either (there is no EXPORT_SYMBOL for it), there is zero chance of
>>>> breaking any use case.
>>>
>>> Don't we still want to keep it around, at least as formal documentation
>>> of the expected status of the list?
>>
>> Hmm, is it really worth the extra code?
> 
> I have already seen such kind of piece of code getting very convenient
> when tracking odd bugs.

What about putting it under CONFIG_TEST then?

This would keep it in the source and it would even be compile tested by
a simple "make testbuild", which is a simple test I'm always doing
before submitting a patch.


Juergen

Jürgen Groß, le jeu. 25 juil. 2024 08:25:18 +0200, a ecrit:
> On 25.07.24 00:44, Samuel Thibault wrote:
> > Hello,
> > 
> > Jürgen Groß, le mar. 23 juil. 2024 08:36:13 +0200, a ecrit:
> > > On 22.07.24 23:35, Samuel Thibault wrote:
> > > > Juergen Gross, le lun. 22 juil. 2024 17:01:41 +0200, a ecrit:
> > > > > Remove the sanity_check() function, as it is used nowhere.
> > > > > 
> > > > > Since any application linked with Mini-OS can't call sanity_check()
> > > > > either (there is no EXPORT_SYMBOL for it), there is zero chance of
> > > > > breaking any use case.
> > > > 
> > > > Don't we still want to keep it around, at least as formal documentation
> > > > of the expected status of the list?
> > > 
> > > Hmm, is it really worth the extra code?
> > 
> > I have already seen such kind of piece of code getting very convenient
> > when tracking odd bugs.
> 
> What about putting it under CONFIG_TEST then?

Ok !

Samuel

On 25.07.24 08:29, Samuel Thibault wrote:
> Jürgen Groß, le jeu. 25 juil. 2024 08:25:18 +0200, a ecrit:
>> On 25.07.24 00:44, Samuel Thibault wrote:
>>> Hello,
>>>
>>> Jürgen Groß, le mar. 23 juil. 2024 08:36:13 +0200, a ecrit:
>>>> On 22.07.24 23:35, Samuel Thibault wrote:
>>>>> Juergen Gross, le lun. 22 juil. 2024 17:01:41 +0200, a ecrit:
>>>>>> Remove the sanity_check() function, as it is used nowhere.
>>>>>>
>>>>>> Since any application linked with Mini-OS can't call sanity_check()
>>>>>> either (there is no EXPORT_SYMBOL for it), there is zero chance of
>>>>>> breaking any use case.
>>>>>
>>>>> Don't we still want to keep it around, at least as formal documentation
>>>>> of the expected status of the list?
>>>>
>>>> Hmm, is it really worth the extra code?
>>>
>>> I have already seen such kind of piece of code getting very convenient
>>> when tracking odd bugs.
>>
>> What about putting it under CONFIG_TEST then?
> 
> Ok !

I went a little bit further by calling sanity_check() from periodic_thread()
in test.c once a second. This will make real use of the function.


Juergen