Message ID | 20240607083711.62833-3-david@redhat.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | mm/highmem: don't track highmem pages manually | expand |
On Fri, Jun 07, 2024 at 10:37:11AM +0200, David Hildenbrand wrote: >It looks rather weird that totalhigh_pages() returns an >"unsigned long" but nr_free_highpages() returns an "unsigned int". > >Let's return an "unsigned long" from nr_free_highpages() to be >consistent. > >While at it, use a plain "0" instead of a "0UL" in the !CONFIG_HIGHMEM >totalhigh_pages() implementation, to make these look alike as well. I am not sure why not use 0UL for both? > >Signed-off-by: David Hildenbrand <david@redhat.com> >--- > include/linux/highmem-internal.h | 8 ++++---- > include/linux/highmem.h | 2 +- > mm/highmem.c | 4 ++-- > 3 files changed, 7 insertions(+), 7 deletions(-) > >diff --git a/include/linux/highmem-internal.h b/include/linux/highmem-internal.h >index 65f865fbbac04..dd100e849f5e0 100644 >--- a/include/linux/highmem-internal.h >+++ b/include/linux/highmem-internal.h >@@ -131,10 +131,10 @@ static inline void __kunmap_atomic(const void *addr) > preempt_enable(); > } > >-unsigned int __nr_free_highpages(void); >+unsigned long __nr_free_highpages(void); > unsigned long __totalhigh_pages(void); > >-static inline unsigned int nr_free_highpages(void) >+static inline unsigned long nr_free_highpages(void) > { > return __nr_free_highpages(); > } >@@ -234,8 +234,8 @@ static inline void __kunmap_atomic(const void *addr) > preempt_enable(); > } > >-static inline unsigned int nr_free_highpages(void) { return 0; } >-static inline unsigned long totalhigh_pages(void) { return 0UL; } >+static inline unsigned long nr_free_highpages(void) { return 0; } >+static inline unsigned long totalhigh_pages(void) { return 0; } > > static inline bool is_kmap_addr(const void *x) > { >diff --git a/include/linux/highmem.h b/include/linux/highmem.h >index 6b0d6f3c8580c..930a591b9b616 100644 >--- a/include/linux/highmem.h >+++ b/include/linux/highmem.h >@@ -179,7 +179,7 @@ static inline void *kmap_local_folio(struct folio *folio, size_t offset); > static inline void *kmap_atomic(struct page *page); > > /* Highmem related interfaces for management code */ >-static inline unsigned int nr_free_highpages(void); >+static inline unsigned long nr_free_highpages(void); > static inline unsigned long totalhigh_pages(void); > > #ifndef ARCH_HAS_FLUSH_ANON_PAGE >diff --git a/mm/highmem.c b/mm/highmem.c >index 3c4e9f8c26dcd..1ece1e69031e7 100644 >--- a/mm/highmem.c >+++ b/mm/highmem.c >@@ -111,10 +111,10 @@ static inline wait_queue_head_t *get_pkmap_wait_queue_head(unsigned int color) > } > #endif > >-unsigned int __nr_free_highpages(void) >+unsigned long __nr_free_highpages(void) > { >+ unsigned long pages = 0; > struct zone *zone; >- unsigned int pages = 0; > > for_each_populated_zone(zone) { > if (is_highmem(zone)) >-- >2.45.1
On Fri, Jun 07, 2024 at 10:37:11AM +0200, David Hildenbrand wrote: > It looks rather weird that totalhigh_pages() returns an > "unsigned long" but nr_free_highpages() returns an "unsigned int". > > Let's return an "unsigned long" from nr_free_highpages() to be > consistent. > > While at it, use a plain "0" instead of a "0UL" in the !CONFIG_HIGHMEM > totalhigh_pages() implementation, to make these look alike as well. > > Signed-off-by: David Hildenbrand <david@redhat.com> ... > -static inline unsigned int nr_free_highpages(void) { return 0; } > -static inline unsigned long totalhigh_pages(void) { return 0UL; } > +static inline unsigned long nr_free_highpages(void) { return 0; } > +static inline unsigned long totalhigh_pages(void) { return 0; } Although I doubt it has any consequences, I would just leave them both with UL, so the return type is consistent with what we are returning. Other than that Reviewed-by: Oscar Salvador <osalvador@suse.de>
On 10.06.24 05:40, Oscar Salvador wrote: > On Fri, Jun 07, 2024 at 10:37:11AM +0200, David Hildenbrand wrote: >> It looks rather weird that totalhigh_pages() returns an >> "unsigned long" but nr_free_highpages() returns an "unsigned int". >> >> Let's return an "unsigned long" from nr_free_highpages() to be >> consistent. >> >> While at it, use a plain "0" instead of a "0UL" in the !CONFIG_HIGHMEM >> totalhigh_pages() implementation, to make these look alike as well. >> >> Signed-off-by: David Hildenbrand <david@redhat.com> > ... >> -static inline unsigned int nr_free_highpages(void) { return 0; } >> -static inline unsigned long totalhigh_pages(void) { return 0UL; } >> +static inline unsigned long nr_free_highpages(void) { return 0; } >> +static inline unsigned long totalhigh_pages(void) { return 0; } > > Although I doubt it has any consequences, I would just leave them both with UL, > so the return type is consistent with what we are returning. These suffixes are only required when using constants that would not fit into the native (int) type, or converting from that native (int) type to something else automatically by the compiler would mess things up (for example, undesired sign extension). For 0 that is certainly impossible :) That's also the reason why in include/linux we now have: t14s: ~/git/linux/include/linux $ git grep "return 0UL;" skbuff.h: return 0UL; uaccess.h:static inline unsigned long user_access_save(void) { return 0UL; } t14s: ~/git/linux/include/linux $ git grep "0UL;" bitmap.h: *dst = ~0UL; dax.h: return ~0UL; mtd/map.h: r.x[i] = ~0UL; netfilter.h: return ((ul1[0] ^ ul2[0]) | (ul1[1] ^ ul2[1])) == 0UL; skbuff.h: return 0UL; uaccess.h:static inline unsigned long user_access_save(void) { return 0UL; } ... compared to a long list if "unsigned long" functions that simply "return 0;" So I prefer to just drop it. Thanks!
On Mon, Jun 10, 2024 at 10:22:49AM +0200, David Hildenbrand wrote: >On 10.06.24 05:40, Oscar Salvador wrote: >> On Fri, Jun 07, 2024 at 10:37:11AM +0200, David Hildenbrand wrote: >> > It looks rather weird that totalhigh_pages() returns an >> > "unsigned long" but nr_free_highpages() returns an "unsigned int". >> > >> > Let's return an "unsigned long" from nr_free_highpages() to be >> > consistent. >> > >> > While at it, use a plain "0" instead of a "0UL" in the !CONFIG_HIGHMEM >> > totalhigh_pages() implementation, to make these look alike as well. >> > >> > Signed-off-by: David Hildenbrand <david@redhat.com> >> ... >> > -static inline unsigned int nr_free_highpages(void) { return 0; } >> > -static inline unsigned long totalhigh_pages(void) { return 0UL; } >> > +static inline unsigned long nr_free_highpages(void) { return 0; } >> > +static inline unsigned long totalhigh_pages(void) { return 0; } >> >> Although I doubt it has any consequences, I would just leave them both with UL, >> so the return type is consistent with what we are returning. > >These suffixes are only required when using constants that would not fit >into the native (int) type, or converting from that native (int) type to >something else automatically by the compiler would mess things up (for example, >undesired sign extension). For 0 that is certainly impossible :) > > >That's also the reason why in include/linux we now have: > >t14s: ~/git/linux/include/linux $ git grep "return 0UL;" >skbuff.h: return 0UL; >uaccess.h:static inline unsigned long user_access_save(void) { return 0UL; } >t14s: ~/git/linux/include/linux $ git grep "0UL;" >bitmap.h: *dst = ~0UL; >dax.h: return ~0UL; >mtd/map.h: r.x[i] = ~0UL; >netfilter.h: return ((ul1[0] ^ ul2[0]) | (ul1[1] ^ ul2[1])) == 0UL; >skbuff.h: return 0UL; >uaccess.h:static inline unsigned long user_access_save(void) { return 0UL; } > > >... compared to a long list if "unsigned long" functions that simply "return 0;" > Seems this is the current status. Then my question is do we have a guide line for this? Or 0 is the special case? Sounds positive value has no sign extension problem. If we need to return 1, we suppose to use 1 or 1UL? I found myself confused. I grepped "return 1" and do find some cases without UL: backing-dev.h: wb_stat_error() return 1 for unsigned long. pgtable.h: pte_batch_hint() return 1 for unsigned int. So the guide line is for positive value, it is not necessary to use UL? > >So I prefer to just drop it. > >Thanks! > >-- >Cheers, > >David / dhildenb
On Tue, Jun 11, 2024 at 11:20:00AM +0200, David Hildenbrand wrote: >On 11.06.24 02:56, Wei Yang wrote: >> On Mon, Jun 10, 2024 at 10:22:49AM +0200, David Hildenbrand wrote: >> > On 10.06.24 05:40, Oscar Salvador wrote: >> > > On Fri, Jun 07, 2024 at 10:37:11AM +0200, David Hildenbrand wrote: >> > > > It looks rather weird that totalhigh_pages() returns an >> > > > "unsigned long" but nr_free_highpages() returns an "unsigned int". >> > > > >> > > > Let's return an "unsigned long" from nr_free_highpages() to be >> > > > consistent. >> > > > >> > > > While at it, use a plain "0" instead of a "0UL" in the !CONFIG_HIGHMEM >> > > > totalhigh_pages() implementation, to make these look alike as well. >> > > > >> > > > Signed-off-by: David Hildenbrand <david@redhat.com> >> > > ... >> > > > -static inline unsigned int nr_free_highpages(void) { return 0; } >> > > > -static inline unsigned long totalhigh_pages(void) { return 0UL; } >> > > > +static inline unsigned long nr_free_highpages(void) { return 0; } >> > > > +static inline unsigned long totalhigh_pages(void) { return 0; } >> > > >> > > Although I doubt it has any consequences, I would just leave them both with UL, >> > > so the return type is consistent with what we are returning. >> > >> > These suffixes are only required when using constants that would not fit >> > into the native (int) type, or converting from that native (int) type to >> > something else automatically by the compiler would mess things up (for example, >> > undesired sign extension). For 0 that is certainly impossible :) >> > >> > >> > That's also the reason why in include/linux we now have: >> > >> > t14s: ~/git/linux/include/linux $ git grep "return 0UL;" >> > skbuff.h: return 0UL; >> > uaccess.h:static inline unsigned long user_access_save(void) { return 0UL; } >> > t14s: ~/git/linux/include/linux $ git grep "0UL;" >> > bitmap.h: *dst = ~0UL; >> > dax.h: return ~0UL; >> > mtd/map.h: r.x[i] = ~0UL; >> > netfilter.h: return ((ul1[0] ^ ul2[0]) | (ul1[1] ^ ul2[1])) == 0UL; >> > skbuff.h: return 0UL; >> > uaccess.h:static inline unsigned long user_access_save(void) { return 0UL; } >> > >> > >> > ... compared to a long list if "unsigned long" functions that simply "return 0;" >> > >> >> Seems this is the current status. >> >> Then my question is do we have a guide line for this? Or 0 is the special >> case? Sounds positive value has no sign extension problem. If we need to >> return 1, we suppose to use 1 or 1UL? I found myself confused. >> >> I grepped "return 1" and do find some cases without UL: >> >> backing-dev.h: wb_stat_error() return 1 for unsigned long. >> pgtable.h: pte_batch_hint() return 1 for unsigned int. >> >> So the guide line is for positive value, it is not necessary to use UL? > >I think when returning simple values (0/1/-1), we really don't need these >suffices at all. The standard says "The type of an integer constant is the >first of the corresponding list in which its value can be represented.". I >thought it would always use an "int", but that is not the case. > >So, if we use "-1", the compiler will use an "int", and sign extension to >"unsigned" long will do the right thing. > >Simple test: > >-1 results in: 0xffffffffffffffff >-1U results in: 0xffffffff >-1UL results in: 0xffffffffffffffff >0xffffffff results in: 0xffffffff >0xffffffffU results in: 0xffffffff >0xffffffffUL results in: 0xffffffff >~0xffffffff results in: 0x0 >~0xffffffffU results in: 0x0 >~0xffffffffUL results in: 0xffffffff00000000 >0xffffffffffffffff results in: 0xffffffffffffffff >0xffffffffffffffffU results in: 0xffffffffffffffff I expect this to be 0xffffffff. Why this extend it to a UL? >0xffffffffffffffffUL results in: 0xffffffffffffffff > > >I thought that "0xffffffff" could be a problem (sign-extending to >0xffffffffffffffff), but that does not seem to be the case -- likely using >"unsigned int" as type. Also, I'm surprised that 0xffffffffffffffffU works as >expected, I would have thought the "U" would make the compiler complain about >the value not fitting into an unsigned int. > > >When only returning values, the compiler usually does the right thing. Only >when performing operations on the constant (see ~ example above), we might >have to use the suffixes, depending on the intended outcome. > Looks the guide line is * no need to put suffix on return value * add suffix when performing operations, like ~, << >-- >Cheers, > >David / dhildenb
On 12.06.24 09:01, Wei Yang wrote: > On Tue, Jun 11, 2024 at 11:20:00AM +0200, David Hildenbrand wrote: >> On 11.06.24 02:56, Wei Yang wrote: >>> On Mon, Jun 10, 2024 at 10:22:49AM +0200, David Hildenbrand wrote: >>>> On 10.06.24 05:40, Oscar Salvador wrote: >>>>> On Fri, Jun 07, 2024 at 10:37:11AM +0200, David Hildenbrand wrote: >>>>>> It looks rather weird that totalhigh_pages() returns an >>>>>> "unsigned long" but nr_free_highpages() returns an "unsigned int". >>>>>> >>>>>> Let's return an "unsigned long" from nr_free_highpages() to be >>>>>> consistent. >>>>>> >>>>>> While at it, use a plain "0" instead of a "0UL" in the !CONFIG_HIGHMEM >>>>>> totalhigh_pages() implementation, to make these look alike as well. >>>>>> >>>>>> Signed-off-by: David Hildenbrand <david@redhat.com> >>>>> ... >>>>>> -static inline unsigned int nr_free_highpages(void) { return 0; } >>>>>> -static inline unsigned long totalhigh_pages(void) { return 0UL; } >>>>>> +static inline unsigned long nr_free_highpages(void) { return 0; } >>>>>> +static inline unsigned long totalhigh_pages(void) { return 0; } >>>>> >>>>> Although I doubt it has any consequences, I would just leave them both with UL, >>>>> so the return type is consistent with what we are returning. >>>> >>>> These suffixes are only required when using constants that would not fit >>>> into the native (int) type, or converting from that native (int) type to >>>> something else automatically by the compiler would mess things up (for example, >>>> undesired sign extension). For 0 that is certainly impossible :) >>>> >>>> >>>> That's also the reason why in include/linux we now have: >>>> >>>> t14s: ~/git/linux/include/linux $ git grep "return 0UL;" >>>> skbuff.h: return 0UL; >>>> uaccess.h:static inline unsigned long user_access_save(void) { return 0UL; } >>>> t14s: ~/git/linux/include/linux $ git grep "0UL;" >>>> bitmap.h: *dst = ~0UL; >>>> dax.h: return ~0UL; >>>> mtd/map.h: r.x[i] = ~0UL; >>>> netfilter.h: return ((ul1[0] ^ ul2[0]) | (ul1[1] ^ ul2[1])) == 0UL; >>>> skbuff.h: return 0UL; >>>> uaccess.h:static inline unsigned long user_access_save(void) { return 0UL; } >>>> >>>> >>>> ... compared to a long list if "unsigned long" functions that simply "return 0;" >>>> >>> >>> Seems this is the current status. >>> >>> Then my question is do we have a guide line for this? Or 0 is the special >>> case? Sounds positive value has no sign extension problem. If we need to >>> return 1, we suppose to use 1 or 1UL? I found myself confused. >>> >>> I grepped "return 1" and do find some cases without UL: >>> >>> backing-dev.h: wb_stat_error() return 1 for unsigned long. >>> pgtable.h: pte_batch_hint() return 1 for unsigned int. >>> >>> So the guide line is for positive value, it is not necessary to use UL? >> >> I think when returning simple values (0/1/-1), we really don't need these >> suffices at all. The standard says "The type of an integer constant is the >> first of the corresponding list in which its value can be represented.". I >> thought it would always use an "int", but that is not the case. >> >> So, if we use "-1", the compiler will use an "int", and sign extension to >> "unsigned" long will do the right thing. >> >> Simple test: >> >> -1 results in: 0xffffffffffffffff >> -1U results in: 0xffffffff >> -1UL results in: 0xffffffffffffffff >> 0xffffffff results in: 0xffffffff >> 0xffffffffU results in: 0xffffffff >> 0xffffffffUL results in: 0xffffffff >> ~0xffffffff results in: 0x0 >> ~0xffffffffU results in: 0x0 >> ~0xffffffffUL results in: 0xffffffff00000000 >> 0xffffffffffffffff results in: 0xffffffffffffffff >> 0xffffffffffffffffU results in: 0xffffffffffffffff > > I expect this to be 0xffffffff. Why this extend it to a UL? Apparently, the "U" only restricts the set of types to "unsigned ones". https://en.cppreference.com/w/cpp/language/integer_literal So the compiler will use the first "unsigned" type that can hold that value.
On Wed, Jun 12, 2024 at 09:22:25AM +0200, David Hildenbrand wrote: >On 12.06.24 09:01, Wei Yang wrote: >> On Tue, Jun 11, 2024 at 11:20:00AM +0200, David Hildenbrand wrote: >> > On 11.06.24 02:56, Wei Yang wrote: >> > > On Mon, Jun 10, 2024 at 10:22:49AM +0200, David Hildenbrand wrote: >> > > > On 10.06.24 05:40, Oscar Salvador wrote: >> > > > > On Fri, Jun 07, 2024 at 10:37:11AM +0200, David Hildenbrand wrote: >> > > > > > It looks rather weird that totalhigh_pages() returns an >> > > > > > "unsigned long" but nr_free_highpages() returns an "unsigned int". >> > > > > > >> > > > > > Let's return an "unsigned long" from nr_free_highpages() to be >> > > > > > consistent. >> > > > > > >> > > > > > While at it, use a plain "0" instead of a "0UL" in the !CONFIG_HIGHMEM >> > > > > > totalhigh_pages() implementation, to make these look alike as well. >> > > > > > >> > > > > > Signed-off-by: David Hildenbrand <david@redhat.com> >> > > > > ... >> > > > > > -static inline unsigned int nr_free_highpages(void) { return 0; } >> > > > > > -static inline unsigned long totalhigh_pages(void) { return 0UL; } >> > > > > > +static inline unsigned long nr_free_highpages(void) { return 0; } >> > > > > > +static inline unsigned long totalhigh_pages(void) { return 0; } >> > > > > >> > > > > Although I doubt it has any consequences, I would just leave them both with UL, >> > > > > so the return type is consistent with what we are returning. >> > > > >> > > > These suffixes are only required when using constants that would not fit >> > > > into the native (int) type, or converting from that native (int) type to >> > > > something else automatically by the compiler would mess things up (for example, >> > > > undesired sign extension). For 0 that is certainly impossible :) >> > > > >> > > > >> > > > That's also the reason why in include/linux we now have: >> > > > >> > > > t14s: ~/git/linux/include/linux $ git grep "return 0UL;" >> > > > skbuff.h: return 0UL; >> > > > uaccess.h:static inline unsigned long user_access_save(void) { return 0UL; } >> > > > t14s: ~/git/linux/include/linux $ git grep "0UL;" >> > > > bitmap.h: *dst = ~0UL; >> > > > dax.h: return ~0UL; >> > > > mtd/map.h: r.x[i] = ~0UL; >> > > > netfilter.h: return ((ul1[0] ^ ul2[0]) | (ul1[1] ^ ul2[1])) == 0UL; >> > > > skbuff.h: return 0UL; >> > > > uaccess.h:static inline unsigned long user_access_save(void) { return 0UL; } >> > > > >> > > > >> > > > ... compared to a long list if "unsigned long" functions that simply "return 0;" >> > > > >> > > >> > > Seems this is the current status. >> > > >> > > Then my question is do we have a guide line for this? Or 0 is the special >> > > case? Sounds positive value has no sign extension problem. If we need to >> > > return 1, we suppose to use 1 or 1UL? I found myself confused. >> > > >> > > I grepped "return 1" and do find some cases without UL: >> > > >> > > backing-dev.h: wb_stat_error() return 1 for unsigned long. >> > > pgtable.h: pte_batch_hint() return 1 for unsigned int. >> > > >> > > So the guide line is for positive value, it is not necessary to use UL? >> > >> > I think when returning simple values (0/1/-1), we really don't need these >> > suffices at all. The standard says "The type of an integer constant is the >> > first of the corresponding list in which its value can be represented.". I >> > thought it would always use an "int", but that is not the case. >> > >> > So, if we use "-1", the compiler will use an "int", and sign extension to >> > "unsigned" long will do the right thing. >> > >> > Simple test: >> > >> > -1 results in: 0xffffffffffffffff >> > -1U results in: 0xffffffff >> > -1UL results in: 0xffffffffffffffff >> > 0xffffffff results in: 0xffffffff >> > 0xffffffffU results in: 0xffffffff >> > 0xffffffffUL results in: 0xffffffff >> > ~0xffffffff results in: 0x0 >> > ~0xffffffffU results in: 0x0 >> > ~0xffffffffUL results in: 0xffffffff00000000 >> > 0xffffffffffffffff results in: 0xffffffffffffffff >> > 0xffffffffffffffffU results in: 0xffffffffffffffff >> >> I expect this to be 0xffffffff. Why this extend it to a UL? > >Apparently, the "U" only restricts the set of types to "unsigned ones". > >https://en.cppreference.com/w/cpp/language/integer_literal > >So the compiler will use the first "unsigned" type that can hold that value. > Interesting, thanks for the reference. Reviewed-by: Wei Yang <richard.weiyang@gmail.com> >-- >Cheers, > >David / dhildenb
diff --git a/include/linux/highmem-internal.h b/include/linux/highmem-internal.h index 65f865fbbac04..dd100e849f5e0 100644 --- a/include/linux/highmem-internal.h +++ b/include/linux/highmem-internal.h @@ -131,10 +131,10 @@ static inline void __kunmap_atomic(const void *addr) preempt_enable(); } -unsigned int __nr_free_highpages(void); +unsigned long __nr_free_highpages(void); unsigned long __totalhigh_pages(void); -static inline unsigned int nr_free_highpages(void) +static inline unsigned long nr_free_highpages(void) { return __nr_free_highpages(); } @@ -234,8 +234,8 @@ static inline void __kunmap_atomic(const void *addr) preempt_enable(); } -static inline unsigned int nr_free_highpages(void) { return 0; } -static inline unsigned long totalhigh_pages(void) { return 0UL; } +static inline unsigned long nr_free_highpages(void) { return 0; } +static inline unsigned long totalhigh_pages(void) { return 0; } static inline bool is_kmap_addr(const void *x) { diff --git a/include/linux/highmem.h b/include/linux/highmem.h index 6b0d6f3c8580c..930a591b9b616 100644 --- a/include/linux/highmem.h +++ b/include/linux/highmem.h @@ -179,7 +179,7 @@ static inline void *kmap_local_folio(struct folio *folio, size_t offset); static inline void *kmap_atomic(struct page *page); /* Highmem related interfaces for management code */ -static inline unsigned int nr_free_highpages(void); +static inline unsigned long nr_free_highpages(void); static inline unsigned long totalhigh_pages(void); #ifndef ARCH_HAS_FLUSH_ANON_PAGE diff --git a/mm/highmem.c b/mm/highmem.c index 3c4e9f8c26dcd..1ece1e69031e7 100644 --- a/mm/highmem.c +++ b/mm/highmem.c @@ -111,10 +111,10 @@ static inline wait_queue_head_t *get_pkmap_wait_queue_head(unsigned int color) } #endif -unsigned int __nr_free_highpages(void) +unsigned long __nr_free_highpages(void) { + unsigned long pages = 0; struct zone *zone; - unsigned int pages = 0; for_each_populated_zone(zone) { if (is_highmem(zone))
It looks rather weird that totalhigh_pages() returns an "unsigned long" but nr_free_highpages() returns an "unsigned int". Let's return an "unsigned long" from nr_free_highpages() to be consistent. While at it, use a plain "0" instead of a "0UL" in the !CONFIG_HIGHMEM totalhigh_pages() implementation, to make these look alike as well. Signed-off-by: David Hildenbrand <david@redhat.com> --- include/linux/highmem-internal.h | 8 ++++---- include/linux/highmem.h | 2 +- mm/highmem.c | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-)